.NET’s SignedXML class has had a risky implementation for lookup of XML elements by id in 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). The fix also contains a number of related breaking changes.

This is an example of a signed XML document with data that might be incorrectly trusted.

<r>
   <b Id="q">
      <data>Valid data</data>
      <Signature xmlns="http://www.w3.org/2000/09/xmldsig#">
         <SignedInfo>
            <CanonicalizationMethod Algorithm="http://www.w3.org/TR/2001/REC-xml-c14n-20010315" />
            <SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" />
            <Reference URI="#q">
               <Transforms>
                  <Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" />
               </Transforms>
               <DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" />
               <DigestValue>Drhn/EC2O7JBZwj9lS/kdS8RYis=</DigestValue>
            </Reference>
         </SignedInfo>
         <SignatureValue>T2hBKCNuonADXznJ2IT/cIH2ZB/8/WvLpywVH3ebWwN9EDKt5T4n4NC7/rhWTFMX3pacGNzS0oDcEe7iYBW05eJou2XGzX+GXD+I8nPE7nXOQzVYZDnN+1tGfn35L1z86iZHyUXZsTwJ1FA9VZk3ph6zCAn5YmBYg495fg2chFI=</SignatureValue>
      </Signature>
   </b>
   <b Id="q">
      <data>Some false data</data>
   </b>
</r>

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 <b> nodes.

This is some typical validation code that will process untrusted data.

// Check signature
var signedXml = new SignedXml(xmlDoc);
signedXml.LoadXml(signatureNode);
Console.WriteLine("Signature is correct: " + signedXml.CheckSignature(key));
 
Console.Write("\n** Checking elements with name b **");
// Loop nodes and check against reference id.
foreach (var e in xmlDoc.SelectNodes("//b").Cast<XmlElement>())
{
  Console.WriteLine("\nMatching reference found in signedinfo: " 
    + signedXml.SignedInfo.References.Cast<Reference>()
      .Any(r => r.Uri == "#" + e.GetAttribute("Id")));
  Console.WriteLine("Contents of element: " + e["data"].InnerText);
}

When run on an unpatched system, it gives the following output:

Signature is correct: True
 
** Checking elements with name b **
Matching reference found in signedinfo: True
Contents of element: Valid data
 
Matching reference found in signedinfo: True
Contents of element: Some false data

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 HKEY_LOCAL_MACHINE\SOFTWARE\Microsoft\.NETFramework\Security named SignedXmlAllowAmbiguousReferenceTargets with value 1.

The Vulnerable Code

The code that lies behind this vulernability 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).

public virtual XmlElement GetIdElement (XmlDocument document, string idValue) {
  if (document == null)
    return null;
 
  // Get the element with idValue
  XmlElement elem = document.GetElementById(idValue);
  if (elem != null)
    return elem;
  elem = document.SelectSingleNode("//*[@Id=\"" + idValue + "\"]") as XmlElement;
  if (elem != null)
    return elem;
  elem = document.SelectSingleNode("//*[@id=\"" + idValue + "\"]") as XmlElement;
  if (elem != null)
    return elem;
  elem = document.SelectSingleNode("//*[@ID=\"" + idValue + "\"]") as XmlElement;
    return elem;
}

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.

Mitigating Factors

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 an 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.

Workarounds

With the old implementation of SignedXml.GetIdElement() two workarounds are suggested.

Resolve References with SignedXml.GetIdElement()

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.

foreach (var r in signedXml.SignedInfo.References.Cast<Reference>())
{
  if (r.Uri[0] == '#')
  {
    var e = x.GetIdElement(xmlDoc, r.Uri.Substring(1));
    Console.WriteLine("Contents of element: " + e.InnerText);
  }
}

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.

Overload SignedXml.GetIdElement()

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.