GetIdElement()when resolving signed XML references. The lookup validated only the first element if there are several with the same id. This opens up for XML Signature Wrapping attacks in any library that is using the default implementation without taking necessary precautions. For SAML2 libraries, signature wrapping is a well-known attack class with very severe implications.
I reported this privately to Microsoft on December 3rd, 2015. They responded (as promised within 24 hours) that they would investigate. The vulnerability was assigned ids CVE-2016-0132 and MS16-035. A fix was released on "patch Tuesday" in March 2016 (and yes, I’m proud to be listed in the acknowledgement section).
This is an example of a signed XML document with data that might be incorrectly trusted.
The document demonstrates how two elements have the same id. The unpatched
SignedXml.GetIdElement() method will only find and validate the first occurrence of the id, but code that loops all nodes and checks that the id is present in the signature’s references will trust both
This is some typical validation code that will process untrusted data.
When run on an unpatched system, it gives the following output:
On a patched system, a
CryptograhpicException is thrown with the message "Malformed reference element." It is possible to disable the checking and revert to the old unsafe behaviour by adding a DWORD value under
SignedXmlAllowAmbiguousReferenceTargets with value 1.
The Vulnerable Code
The code that lies behind this vulnerability is in the
SignedXml.GetIdElement() method. This is the listing of the method in the reference source (retrieved 2016-03-10, .NET 4.6.1; it is still the old code and not the updated of the patch).
The problem is the usage of
SelectSingleNode, which is what opens up for the duplicate id attack. For anyone being used to LINQ it has a deceiving name. Following the LINQ conventions, it should be named
SelectFirstNodeOrDefault. The signature will validate the first matching node and completely ignore if there are other nodes with the same id in the document.
The ability to successfully exploit the vulnerability depends entirely on if the document format allows duplicate ids to be inserted. In the SAML2 case, the risk is reduced by the fact that an enveloped signature is used and the id to be signed is the one of the enclosing element. As long as the signed element is the root of the document, the attack is not possible. If several assertions are allowed, a secondary assertion might be injected with a signature copied from the first assertion. Or the assertion validated by the signature could be placed in an extensions element before the assertion. My assessment is that an attack is possible for many implementations of SAML2 and other protocols. By sheer luck, Kentor.AuthServices has never been vulnerable to this attack, as the XML of each assertion was extracted to its own
XmlDocument before signature validation. The signature validation code was updated in 0.16.0 and by then, I had found this vulnerability and applied a workaround.
With the old implementation of
SignedXml.GetIdElement() two workarounds are suggested.
Resolve References With
The first workaround is to use
SignedXml.GetIdElement() to resolve the references in the calling code. This guarantees that only exactly the same data as validated by the signature will be processed. This is what Kentor.AuthServices 0.16.0 uses.
While using this approach is safe, there are drawbacks as the context of the signed element is lost. The returned element is for sure the one that is signed, but it could be anywhere in the enclosing XML document.
Another alternative is to overload the
SignedXml.GetIdElement() method with a better implementation that checks if there are multiple elements with the same id. This is what the patched version from Microsoft does.