fix: don't panic on quoted Reference URIs

signedxml.go

@@ -257,32 +257,16 @@ func (s *signatureData) getReferencedXML(reference *etree.Element, inputDoc *etr
 	uri = strings.Replace(uri, "#", "", 1)
 	// populate doc with the referenced xml from the Reference URI
 	if uri == "" {
-		outputDoc = inputDoc
-	} else {
-		refIDAttribute := "ID"
-		if s.refIDAttribute != "" {
-			refIDAttribute = s.refIDAttribute
-		}
-		path := fmt.Sprintf(".//[@%s='%s']", refIDAttribute, uri)
-		e := inputDoc.FindElement(path)
-		if e != nil {
-			outputDoc = etree.NewDocument()
-			outputDoc.SetRoot(e.Copy())
-		} else {
-			// SAML v1.1 Assertions use AssertionID
-			path := fmt.Sprintf(".//[@AssertionID='%s']", uri)
-			e := inputDoc.FindElement(path)
-			if e != nil {
-				outputDoc = etree.NewDocument()
-				outputDoc.SetRoot(e.Copy())
-			}
-		}
+		return inputDoc, nil
 	}
 
-	if outputDoc == nil {
+	e, err := s.findReferencedElement(reference, inputDoc)
+	if err != nil {
 		return nil, errors.New("signedxml: unable to find refereced xml")
 	}
 
+	outputDoc = etree.NewDocument()
+	outputDoc.SetRoot(e.Copy())
 	return outputDoc, nil
 }
 
@@ -301,22 +285,52 @@ func (s *signatureData) findReferencedElement(reference *etree.Element, inputDoc
 	if s.refIDAttribute != "" {
 		refIDAttribute = s.refIDAttribute
 	}
-	path := fmt.Sprintf(".//[@%s='%s']", refIDAttribute, uri)
-	e := inputDoc.FindElement(path)
-	if e != nil {
+	if e := findElementByAttr(inputDoc.Root(), refIDAttribute, uri); e != nil {
 		return e, nil
 	}
 
 	// SAML v1.1 Assertions use AssertionID
-	path = fmt.Sprintf(".//[@AssertionID='%s']", uri)
-	e = inputDoc.FindElement(path)
-	if e != nil {
+	if e := findElementByAttr(inputDoc.Root(), "AssertionID", uri); e != nil {
 		return e, nil
 	}
 
 	return nil, errors.New("signedxml: unable to find referenced xml element")
 }
 
+// findElementByAttr returns the first element with a matching attribute name
+// and value. Values are compared directly instead of interpolating them into
+// an etree path, which panics on quotes.
+func findElementByAttr(root *etree.Element, attrName, attrValue string) *etree.Element {
+	if root == nil {
+		return nil
+	}
+
+	space, key := "", attrName
+	if i := strings.IndexByte(attrName, ':'); i >= 0 {
+		space, key = attrName[:i], attrName[i+1:]
+	}
+
+	if elementHasAttr(root, space, key, attrValue) {
+		return root
+	}
+	for _, child := range root.ChildElements() {
+		if found := findElementByAttr(child, attrName, attrValue); found != nil {
+			return found
+		}
+	}
+	return nil
+}
+
+func elementHasAttr(e *etree.Element, space, key, value string) bool {
+	for _, a := range e.Attr {
+		// Empty space matches any namespace, matching etree path [@attr='val'].
+		if (space == "" || space == a.Space) && a.Key == key && a.Value == value {
+			return true
+		}
+	}
+	return false
+}
+
 func getCertFromPEMString(pemString string) (*x509.Certificate, error) {
 	// The X509Certificate element contains base64-encoded DER certificate data
 	certPEM := fmt.Sprintf("-----BEGIN CERTIFICATE-----\n%s\n-----END CERTIFICATE-----",

signedxml_test.go

@@ -10,6 +10,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/beevik/etree"
@@ -486,3 +487,54 @@ func TestRSAPSSRoundtrip(t *testing.T) {
 		So(err, ShouldBeNil)
 	})
 }
+
+func TestReferenceURIWithQuoteDoesNotPanic(t *testing.T) {
+	// Fuzz crash: interpolating a Reference URI containing a quote into an
+	// etree path made FindElement panic with "mismatched filter quotes".
+	doc := etree.NewDocument()
+	if err := doc.ReadFromString(`<root ID="abc"/>`); err != nil {
+		t.Fatal(err)
+	}
+	ref := etree.NewElement("Reference")
+	ref.CreateAttr("URI", "#ab'c")
+
+	s := &signatureData{xml: doc}
+	if _, err := s.findReferencedElement(ref, doc); err == nil {
+		t.Fatal("expected error for unmatched quoted URI")
+	}
+	if _, err := s.getReferencedXML(ref, doc); err == nil {
+		t.Fatal("expected error for unmatched quoted URI")
+	}
+
+	quoted := etree.NewDocument()
+	if err := quoted.ReadFromString(`<root ID="ab'c"/>`); err != nil {
+		t.Fatal(err)
+	}
+	elem, err := s.findReferencedElement(ref, quoted)
+	if err != nil {
+		t.Fatalf("expected to find ID with quote: %v", err)
+	}
+	if elem == nil || elem.SelectAttrValue("ID", "") != "ab'c" {
+		t.Fatalf("unexpected element: %#v", elem)
+	}
+}
+
+func TestValidatorQuotedReferenceURI(t *testing.T) {
+	xml, err := os.ReadFile("./testdata/bbauth-metadata.xml")
+	if err != nil {
+		t.Fatal(err)
+	}
+	mutated := strings.Replace(string(xml),
+		`URI="#_69b42076-409e-4476-af41-339962e49427"`,
+		`URI="#_69b42076-409e-4476-af41-339962e49'27"`,
+		1)
+
+	v, err := NewValidator(mutated)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = v.ValidateReferences()
+	if err == nil {
+		t.Fatal("expected validation error for quoted Reference URI")
+	}
+}

testdata/fuzz/FuzzValidator/9f0b5d517ccd5fef

@@ -0,0 +1,2 @@
+go test fuzz v1
+string("<EntityDescriptor ID=\"_69b42076-409e-4476-af41-339962e49427\" entityID=\"Blackbaud Authentication Service\" xmlns=\"urn:oasis:names:tc:SAML:2.0:metadata\"><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/2001/04/xmldsig-more#rsa-sha256\"/><Reference URI=\"#_69b42076-409e-4476-af41-339962e49'27\"><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/2001/04/xmlenc#sha256\"/><DigestValue>LPHoiAkLmA/TGIuVbgpwlFLXL+ymEBc7TS0fC9/PTQU=</DigestValue></Reference></SignedInfo><SignatureValue>d2CXq9GEeDKvMxdpxtTRKQ8TGeSWhJOVPs8LMD0ObeE1t/YGiAm9keorMiki4laxbWqAuOmwHK3qNHogRFgkIYi3fnuFBzMrahXf0n3A5PRXXW1m768Z92GKV09pGuygKUXCtXzwq0seDi6PnzMCJFzFXGQWnum0paa8Oz+6425Sn0zO0fT3ttp3AXeXGyNXwYPYcX1iEMB7klUlyiz2hmn8ngCIbTkru7uIeyPmQ5WD4SS/qQaL4yb3FZibXoe/eRXrbkG1NAJCw9OWw0jsvWncE1rKFaqEMbz21fXSDhh3Ls+p9yVf+xbCrpkT0FMqjTHpNRvccMPZe/hDGrHV7Q==</SignatureValue><KeyInfo><X509Data><X509Certificate>MIIDNzCCAh+gAwIBAgIQQVK+d/vLK4ZNMDk15HGUoTANBgkqhkiG9w0BAQ0FADAoMSYwJAYDVQQDEx1CbGFja2JhdWQgQXV0aGVudGljYXRpb24gMjAyMjAeFw0wMDAxMDEwNDAwMDBaFw0yMjAxMDEwNDAwMDBaMCgxJjAkBgNVBAMTHUJsYWNrYmF1ZCBBdXRoZW50aWNhdGlvbiAyMDIyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArgByjSPVvP4DLf/l7QRz7G7Dhkdns0QjWslnWejHlFIezfkJ4NGPp0+5CRCFYBqAb7DhqyK77Ek5xdzmwgYb1X6GD6UDltWvN5BBFAw69I6/K0WjguFUxk19T7xdc8vTCNAMi+6Ys49O3EBNnI2fiqDoBdMjUTud1F04QY3N2rZWkjMrHV+CnzhoUwqsO/ABWrDbkPzBXdOOIbsKH0k0IP8q2+35pe1y2nxtB9f1fCyCmbUH2HINMHahDmxxanTW5Jy14yD/HSRTFQF9JMTeglomWq5q9VPx0NjsEJR+B5IkRCTf75LoYrrr/fvQm3aummmYPdHauXCBrcm0moX4ywIDAQABo10wWzBZBgNVHQEEUjBQgBDCHOfardZfhltQSbLqsukZoSowKDEmMCQGA1UEAxMdQmxhY2tiYXVkIEF1dGhlbnRpY2F0aW9uIDIwMjKCEEFSvnf7yyuGTTA5NeRxlKEwDQYJKoZIhvcNAQENBQADggEBADrOhfRiynRKGD7EHohpPrltFScJ9+QErYMhEvteqh3C48T99uKgDY8wTqv+PI08QUSZuhmmF2d+W7aRBo3t8ZZepIXCwDaKo/oUp2h5Y9O3vyGDguq5ptgDTmPNYDCwWtdt0TtQYeLtCQTJVbYByWL0eT+KdzQOkAi48cPEOObSc9Biga7LTCcbCVPeJlYzmHDQUhzBt2jcy5BGvmZloI5SsoZvve6ug74qNq8IJMyzJzUp3kRuB0ruKIioSDi1lc783LDT3LSXyIbOGw/vHBEBY4Ax7FK8CqXJ2TsYqVsyo8QypqXDnveLcgK+PNEAhezhxC9hyV8j1I8pfF72ABE=</X509Certificate></X509Data></KeyInfo></Signature><RoleDescriptor xsi:type=\"fed:SecurityTokenServiceType\" protocolSupportEnumeration=\"http://docs.oasis-open.org/wsfed/federation/200706\" xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" xmlns:fed=\"http://docs.oasis-open.org/wsfed/federation/200706\"><KeyDescriptor use=\"signing\"><KeyInfo xmlns=\"http://www.w3.org/2000/09/xmldsig#\"><X509Data><X509Certificate>MIIDNzCCAh+gAwIBAgIQQVK+d/vLK4ZNMDk15HGUoTANBgkqhkiG9w0BAQ0FADAoMSYwJAYDVQQDEx1CbGFja2JhdWQgQXV0aGVudGljYXRpb24gMjAyMjAeFw0wMDAxMDEwNDAwMDBaFw0yMjAxMDEwNDAwMDBaMCgxJjAkBgNVBAMTHUJsYWNrYmF1ZCBBdXRoZW50aWNhdGlvbiAyMDIyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArgByjSPVvP4DLf/l7QRz7G7Dhkdns0QjWslnWejHlFIezfkJ4NGPp0+5CRCFYBqAb7DhqyK77Ek5xdzmwgYb1X6GD6UDltWvN5BBFAw69I6/K0WjguFUxk19T7xdc8vTCNAMi+6Ys49O3EBNnI2fiqDoBdMjUTud1F04QY3N2rZWkjMrHV+CnzhoUwqsO/ABWrDbkPzBXdOOIbsKH0k0IP8q2+35pe1y2nxtB9f1fCyCmbUH2HINMHahDmxxanTW5Jy14yD/HSRTFQF9JMTeglomWq5q9VPx0NjsEJR+B5IkRCTf75LoYrrr/fvQm3aummmYPdHauXCBrcm0moX4ywIDAQABo10wWzBZBgNVHQEEUjBQgBDCHOfardZfhltQSbLqsukZoSowKDEmMCQGA1UEAxMdQmxhY2tiYXVkIEF1dGhlbnRpY2F0aW9uIDIwMjKCEEFSvnf7yyuGTTA5NeRxlKEwDQYJKoZIhvcNAQENBQADggEBADrOhfRiynRKGD7EHohpPrltFScJ9+QErYMhEvteqh3C48T99uKgDY8wTqv+PI08QUSZuhmmF2d+W7aRBo3t8ZZepIXCwDaKo/oUp2h5Y9O3vyGDguq5ptgDTmPNYDCwWtdt0TtQYeLtCQTJVbYByWL0eT+KdzQOkAi48cPEOObSc9Biga7LTCcbCVPeJlYzmHDQUhzBt2jcy5BGvmZloI5SsoZvve6ug74qNq8IJMyzJzUp3kRuB0ruKIioSDi1lc783LDT3LSXyIbOGw/vHBEBY4Ax7FK8CqXJ2TsYqVsyo8QypqXDnveLcgK+PNEAhezhxC9hyV8j1I8pfF72ABE=</X509Certificate></X509Data></KeyInfo></KeyDescriptor><fed:ClaimTypesOffered><auth:ClaimType Uri=\"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress\" xmlns:auth=\"http://docs.oasis-open.org/wsfed/authorization/200706\"><auth:DisplayName>Email</auth:DisplayName><auth:Description>The users email address</auth:Description></auth:ClaimType><auth:ClaimType Uri=\"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier\" xmlns:auth=\"http://docs.oasis-open.org/wsfed/authorization/200706\"><auth:DisplayName>User Identifier</auth:DisplayName><auth:Description>The Id of the user</auth:Description></auth:ClaimType><auth:ClaimType Uri=\"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/givenname\" xmlns:auth=\"http://docs.oasis-open.org/wsfed/authorization/200706\"><auth:DisplayName>First name</auth:DisplayName><auth:Description>The first name of the user</auth:Description></auth:ClaimType><auth:ClaimType Uri=\"http://schemas.xmlsoap.org/ws/2005/05/identity/claims/surname\" xmlns:auth=\"http://docs.oasis-open.org/wsfed/authorization/200706\"><auth:DisplayName>Last name</auth:DisplayName><auth:Description>The last name of the user</auth:Description></auth:ClaimType></fed:ClaimTypesOffered><fed:SecurityTokenServiceEndpoint><wsa:EndpointReference xmlns:wsa=\"http://www.w3.org/2005/08/addressing\"><wsa:Address>https://signin.blackbaud.com/WSFEDERATION/ACTION</wsa:Address></wsa:EndpointReference></fed:SecurityTokenServiceEndpoint><fed:PassiveRequestorEndpoint><wsa:EndpointReference xmlns:wsa=\"http://www.w3.org/2005/08/addressing\"><wsa:Address>https://signin.blackbaud.com/WSFEDERATION/ACTION</wsa:Address></wsa:EndpointReference></fed:PassiveRequestorEndpoint></RoleDescriptor></EntityDescriptor>\n")