Introduction

I originally stumbled on this library during a code review of a third-party product where it was used. When reading the description of the library on GitHub something caught my eye. It said the library was based on this project : https://github.com/onelogin/dotnet-saml. That project displays a very significant warning.

This project was a proof of concept, not recommended to use it in production environments since it not cover all security checks that SAML demand.

So how much of this broken implementation was fixed ? Well, if we look at the commit of the main file, only two things have been fixed (the expiration check and one variant of bogus assertion injection). For those that know SAML, that’s not a lot. SAML is a complicated beast and getting an implementation right is honestly not an easy task. Claiming that it’s a production-ready library is quite a stretch.

With that said, I reported the following vulnerabilities to the maintainer and the issue was closed after fixing only 1 of the 3 issues reported. ¯\_(ツ)_/¯

The vulnerabilities

Assertion injection (Patched)

The way assertions value are fetched is open to a good number of attacks. The main problem is that if the first assertion doesn’t contain the expected attribute, it can end up being fetched from another assertion. Ex.:

When using the AspNetSaml library, the value that will be obtained from “GetEmail” is “foo@example.org” and the “IsValid” function will return true. That’s because the XPath query used (/samlp:Response/saml:Assertion/saml:AttributeStatement/saml:Attribute[@Name=’User.email’]/saml:AttributeValue) returns the first Attribute with the name “User.email”. It doesn’t ensure that the assertion it’s loading the data from is the one that got validated.

Signature relocation (Not patched)

The first step of the signature validation is to retrieve the content of the assertion without the “ds:Signature” tag. Since the “ds:Signature” tag is ignored, it can be relocated inside the content of an “AttributeValue” without voiding the content it signs. When the content of the “AttributeValue” is fetched, it will also contain everything the “ds:Signature” tag contains. This can be used to corrupt or append data to a targeted attribute. Ex :

In this example, the value obtained from “GetEmail” will also contain everything inside the “Signature” tag and the “Assertion” will still be considered to be correctly signed.

Audience mismatch (Not patched)

The library also doesn’t provide any way to validate the “Audience” field. This can be quite useful to exploit the first issue of assertion injection. You just have to get your hand on a signed assertion that’s for another service and that doesn’t contain the same attribute. As we can forge any missing attribute, you’ll just have to forge the expected attribute.

Even with the first vulnerability patched, this is still an issue for SSO that sign assertion for different services. If, for example, you have a user that’s allowed to log in to service A, but not service B. This user can use it’s signed assertion of service A to log in with service B.

Fixing this ?

There are fundamentally a couple of issues with the minimalist implementation that’s being used. Addressing those issues to have a proper SAML implementation would require a significant rewrite of the library.

The schema is never validated. This means you can append additional tag outside of the signed Assertation. The content will still be valid and could interfere with the subsequent XPath query.

The library uses exclusively XPath query on the full content to retrieve data after the assertation has been validated. This has been the root cause of a large amount of significant vulnerability in SAML implementation in the past.

The library doesn’t validate every field of the SAML response. Some field like “NotBefore” and “Audience” are not even looked at. This library is essentially a partial implementation of SAML.

The vulnerabilities found are year-old known issue with SAML. That means that future SAML vulnerabilities are probably going to take a long time to be found and fixed.

At this point the library kind of needs to be completely replaced. Using a broken library as a foundation was already a bad idea to start with. There are a few alternatives linked in the README of the One Login project (https://github.com/onelogin/dotnet-saml) that look good. It is highly recommended to migrate to one of them if you are using AspNetSaml.