diff --git a/signedxml.go b/signedxml.go
index a93c290..3f0a45e 100644
--- a/signedxml.go
+++ b/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-----",
diff --git a/signedxml_test.go b/signedxml_test.go
index 2c35b85..9bc5f1b 100644
--- a/signedxml_test.go
+++ b/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(``); 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(``); 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")
+ }
+}
diff --git a/testdata/fuzz/FuzzValidator/9f0b5d517ccd5fef b/testdata/fuzz/FuzzValidator/9f0b5d517ccd5fef
new file mode 100644
index 0000000..829c39e
--- /dev/null
+++ b/testdata/fuzz/FuzzValidator/9f0b5d517ccd5fef
@@ -0,0 +1,2 @@
+go test fuzz v1
+string("LPHoiAkLmA/TGIuVbgpwlFLXL+ymEBc7TS0fC9/PTQU=d2CXq9GEeDKvMxdpxtTRKQ8TGeSWhJOVPs8LMD0ObeE1t/YGiAm9keorMiki4laxbWqAuOmwHK3qNHogRFgkIYi3fnuFBzMrahXf0n3A5PRXXW1m768Z92GKV09pGuygKUXCtXzwq0seDi6PnzMCJFzFXGQWnum0paa8Oz+6425Sn0zO0fT3ttp3AXeXGyNXwYPYcX1iEMB7klUlyiz2hmn8ngCIbTkru7uIeyPmQ5WD4SS/qQaL4yb3FZibXoe/eRXrbkG1NAJCw9OWw0jsvWncE1rKFaqEMbz21fXSDhh3Ls+p9yVf+xbCrpkT0FMqjTHpNRvccMPZe/hDGrHV7Q==MIIDNzCCAh+gAwIBAgIQQVK+d/vLK4ZNMDk15HGUoTANBgkqhkiG9w0BAQ0FADAoMSYwJAYDVQQDEx1CbGFja2JhdWQgQXV0aGVudGljYXRpb24gMjAyMjAeFw0wMDAxMDEwNDAwMDBaFw0yMjAxMDEwNDAwMDBaMCgxJjAkBgNVBAMTHUJsYWNrYmF1ZCBBdXRoZW50aWNhdGlvbiAyMDIyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArgByjSPVvP4DLf/l7QRz7G7Dhkdns0QjWslnWejHlFIezfkJ4NGPp0+5CRCFYBqAb7DhqyK77Ek5xdzmwgYb1X6GD6UDltWvN5BBFAw69I6/K0WjguFUxk19T7xdc8vTCNAMi+6Ys49O3EBNnI2fiqDoBdMjUTud1F04QY3N2rZWkjMrHV+CnzhoUwqsO/ABWrDbkPzBXdOOIbsKH0k0IP8q2+35pe1y2nxtB9f1fCyCmbUH2HINMHahDmxxanTW5Jy14yD/HSRTFQF9JMTeglomWq5q9VPx0NjsEJR+B5IkRCTf75LoYrrr/fvQm3aummmYPdHauXCBrcm0moX4ywIDAQABo10wWzBZBgNVHQEEUjBQgBDCHOfardZfhltQSbLqsukZoSowKDEmMCQGA1UEAxMdQmxhY2tiYXVkIEF1dGhlbnRpY2F0aW9uIDIwMjKCEEFSvnf7yyuGTTA5NeRxlKEwDQYJKoZIhvcNAQENBQADggEBADrOhfRiynRKGD7EHohpPrltFScJ9+QErYMhEvteqh3C48T99uKgDY8wTqv+PI08QUSZuhmmF2d+W7aRBo3t8ZZepIXCwDaKo/oUp2h5Y9O3vyGDguq5ptgDTmPNYDCwWtdt0TtQYeLtCQTJVbYByWL0eT+KdzQOkAi48cPEOObSc9Biga7LTCcbCVPeJlYzmHDQUhzBt2jcy5BGvmZloI5SsoZvve6ug74qNq8IJMyzJzUp3kRuB0ruKIioSDi1lc783LDT3LSXyIbOGw/vHBEBY4Ax7FK8CqXJ2TsYqVsyo8QypqXDnveLcgK+PNEAhezhxC9hyV8j1I8pfF72ABE=MIIDNzCCAh+gAwIBAgIQQVK+d/vLK4ZNMDk15HGUoTANBgkqhkiG9w0BAQ0FADAoMSYwJAYDVQQDEx1CbGFja2JhdWQgQXV0aGVudGljYXRpb24gMjAyMjAeFw0wMDAxMDEwNDAwMDBaFw0yMjAxMDEwNDAwMDBaMCgxJjAkBgNVBAMTHUJsYWNrYmF1ZCBBdXRoZW50aWNhdGlvbiAyMDIyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEArgByjSPVvP4DLf/l7QRz7G7Dhkdns0QjWslnWejHlFIezfkJ4NGPp0+5CRCFYBqAb7DhqyK77Ek5xdzmwgYb1X6GD6UDltWvN5BBFAw69I6/K0WjguFUxk19T7xdc8vTCNAMi+6Ys49O3EBNnI2fiqDoBdMjUTud1F04QY3N2rZWkjMrHV+CnzhoUwqsO/ABWrDbkPzBXdOOIbsKH0k0IP8q2+35pe1y2nxtB9f1fCyCmbUH2HINMHahDmxxanTW5Jy14yD/HSRTFQF9JMTeglomWq5q9VPx0NjsEJR+B5IkRCTf75LoYrrr/fvQm3aummmYPdHauXCBrcm0moX4ywIDAQABo10wWzBZBgNVHQEEUjBQgBDCHOfardZfhltQSbLqsukZoSowKDEmMCQGA1UEAxMdQmxhY2tiYXVkIEF1dGhlbnRpY2F0aW9uIDIwMjKCEEFSvnf7yyuGTTA5NeRxlKEwDQYJKoZIhvcNAQENBQADggEBADrOhfRiynRKGD7EHohpPrltFScJ9+QErYMhEvteqh3C48T99uKgDY8wTqv+PI08QUSZuhmmF2d+W7aRBo3t8ZZepIXCwDaKo/oUp2h5Y9O3vyGDguq5ptgDTmPNYDCwWtdt0TtQYeLtCQTJVbYByWL0eT+KdzQOkAi48cPEOObSc9Biga7LTCcbCVPeJlYzmHDQUhzBt2jcy5BGvmZloI5SsoZvve6ug74qNq8IJMyzJzUp3kRuB0ruKIioSDi1lc783LDT3LSXyIbOGw/vHBEBY4Ax7FK8CqXJ2TsYqVsyo8QypqXDnveLcgK+PNEAhezhxC9hyV8j1I8pfF72ABE=EmailThe users email addressUser IdentifierThe Id of the userFirst nameThe first name of the userLast nameThe last name of the userhttps://signin.blackbaud.com/WSFEDERATION/ACTIONhttps://signin.blackbaud.com/WSFEDERATION/ACTION\n")