CVE-2023-45683: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: github.com/crewjam/saml is a saml library for the go language. In affected versions the package does not validate the ACS Location URI according to the SAML binding being parsed. If abused, this flaw allows attackers to register malicious Service Providers at the IdP and inject Javascript in the ACS endpoint definition, achieving Cross-Site-Scripting (XSS) in the IdP context during the redirection at the end of a SAML SSO Flow. Consequently, an attacker may perform any authenticated action as the victim once the victim’s browser loaded the SAML IdP initiated SSO link for the malicious service provider. Note: SP registration is commonly an unrestricted operation in IdPs, hence not requiring particular permissions or publicly accessible to ease the IdP interoperability. This issue is fixed in version 0.4.14. Users unable to upgrade may perform external validation of URLs provided in SAML metadata, or restrict the ability for end-users to upload arbitrary metadata.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2023-10-16T19:15Z
Repository: https://github.com/crewjam/saml
Language: Go
Files changed: metadata.go, metadata_test.go, testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml
Commit date: 2023-10-14 10:18:46 -0400
Commit message: Merge pull request from GHSA-267v-3v32-g6q5


Diff: diff --git a/metadata.go b/metadata.go
index fb39a7b..006a9e6 100644
--- a/metadata.go
+++ b/metadata.go
@@ -1,9 +1,11 @@
 package saml
 
 import (
 	"encoding/xml"
+	"fmt"
+	"net/url"
 	"time"
 
 	"github.com/beevik/etree"
 )
 
@@ -17,10 +19,13 @@ const HTTPRedirectBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect"
 const HTTPArtifactBinding = "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Artifact"
 
 // SOAPBinding is the official URN for the SOAP binding (transport)
 const SOAPBinding = "urn:oasis:names:tc:SAML:2.0:bindings:SOAP"
 
+// SOAPBindingV1 is the URN for the SOAP binding in SAML 1.0
+const SOAPBindingV1 = "urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding"
+
 // EntitiesDescriptor represents the SAML object of the same name.
 //
 // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.3.1
 type EntitiesDescriptor struct {
 	XMLName             xml.Name       `xml:"urn:oasis:names:tc:SAML:2.0:metadata EntitiesDescriptor"`
@@ -186,10 +191,80 @@ type Endpoint struct {
 	Binding          string `xml:"Binding,attr"`
 	Location         string `xml:"Location,attr"`
 	ResponseLocation string `xml:"ResponseLocation,attr,omitempty"`
 }
 
+func checkEndpointLocation(binding string, location string) (string, error) {
+	// Within the SAML standard, the complex type EndpointType describes a
+	// SAML protocol binding endpoint at which a SAML entity can be sent
+	// protocol messages. In particular, the location of an endpoint type is
+	// defined as follows in the Metadata for the OASIS Security Assertion
+	// Markup Language (SAML) V2.0 - 2.2.2 Complex Type EndpointType:
+	//
+	//   Location [Required] A required URI attribute that specifies the
+	//   location of the endpoint. The allowable syntax of this URI depends
+	//   on the protocol binding.
+	switch binding {
+	case HTTPPostBinding,
+		HTTPRedirectBinding,
+		HTTPArtifactBinding,
+		SOAPBinding,
+		SOAPBindingV1:
+		locationURL, err := url.Parse(location)
+		if err != nil {
+			return "", fmt.Errorf("invalid url %q: %w", location, err)
+		}
+		switch locationURL.Scheme {
+		case "http", "https":
+		// ok
+		default:
+			return "", fmt.Errorf("invalid url scheme %q for binding %q",
+				locationURL.Scheme, binding)
+		}
+	default:
+		// We don't know what form location should take, but the protocol
+		// requires that we validate its syntax.
+		//
+		// In practice, lots of metadata contains random bindings, for example
+		// "urn:mace:shibboleth:1.0:profiles:AuthnRequest" from our own test suite.
+		//
+		// We can't fail, but we also can't allow a location parameter whose syntax we
+		// cannot verify. The least-bad course of action here is to set location to
+		// and empty string, and hope the caller doesn't care need it.
+		location = ""
+	}
+
+	return location, nil
+}
+
+// UnmarshalXML implements xml.Unmarshaler
+func (m *Endpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+	type Alias Endpoint
+	aux := &struct {
+		*Alias
+	}{
+		Alias: (*Alias)(m),
+	}
+	if err := d.DecodeElement(aux, &start); err != nil {
+		return err
+	}
+
+	var err error
+	m.Location, err = checkEndpointLocation(m.Binding, m.Location)
+	if err != nil {
+		return err
+	}
+	if m.ResponseLocation != "" {
+		m.ResponseLocation, err = checkEndpointLocation(m.Binding, m.ResponseLocation)
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // IndexedEndpoint represents the SAML IndexedEndpointType object.
 //
 // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.2.3
 type IndexedEndpoint struct {
 	Binding          string  `xml:"Binding,attr"`
@@ -197,10 +272,42 @@ type IndexedEndpoint struct {
 	ResponseLocation *string `xml:"ResponseLocation,attr,omitempty"`
 	Index            int     `xml:"index,attr"`
 	IsDefault        *bool   `xml:"isDefault,attr"`
 }
 
+// UnmarshalXML implements xml.Unmarshaler
+func (m *IndexedEndpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
+	type Alias IndexedEndpoint
+	aux := &struct {
+		*Alias
+	}{
+		Alias: (*Alias)(m),
+	}
+	if err := d.DecodeElement(aux, &start); err != nil {
+		return err
+	}
+
+	var err error
+	m.Location, err = checkEndpointLocation(m.Binding, m.Location)
+	if err != nil {
+		return err
+	}
+	if m.ResponseLocation != nil {
+		responseLocation, err := checkEndpointLocation(m.Binding, *m.ResponseLocation)
+		if err != nil {
+			return err
+		}
+		if responseLocation != "" {
+			m.ResponseLocation = &responseLocation
+		} else {
+			m.ResponseLocation = nil
+		}
+	}
+
+	return nil
+}
+
 // SSODescriptor represents the SAML complex type SSODescriptor
 //
 // See http://docs.oasis-open.org/security/saml/v2.0/saml-metadata-2.0-os.pdf §2.4.2
 type SSODescriptor struct {
 	RoleDescriptor
diff --git a/metadata_test.go b/metadata_test.go
index cf1a465..92c7dae 100644
--- a/metadata_test.go
+++ b/metadata_test.go
@@ -163,5 +163,13 @@ cvCsEFiJZ4AbF+DgmO6TarJ8O05t8zvnOwJlNCASPZRH/JmF8tX0hoHuAQ==`,
 
 	buf, err := xml.MarshalIndent(metadata, "", "  ")
 	assert.Check(t, err)
 	golden.Assert(t, string(buf), "TestCanProduceSPMetadata_expected")
 }
+
+func TestMetadataValidatesUrlSchemeForProtocolBinding(t *testing.T) {
+	buf := golden.Get(t, "TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml")
+
+	metadata := EntityDescriptor{}
+	err := xml.Unmarshal(buf, &metadata)
+	assert.Error(t, err, "invalid url scheme \"javascript\" for binding \"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\"")
+}
diff --git a/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml b/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml
new file mode 100644
index 0000000..8273c6b
--- /dev/null
+++ b/testdata/TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml
@@ -0,0 +1 @@
+<?xml version='1.0' encoding='UTF-8'?><md:EntityDescriptor ID='_af805d1c-c2e3-444e-9cf5-efc664eeace6' entityID='https://dev.aa.kndr.org/users/auth/saml/metadata' validUntil='2001-02-03T04:05:06.789' cacheDuration='PT1H' xmlns:md='urn:oasis:names:tc:SAML:2.0:metadata' xmlns:saml='urn:oasis:names:tc:SAML:2.0:assertion'><md:SPSSODescriptor AuthnRequestsSigned='false' WantAssertionsSigned='false' protocolSupportEnumeration='urn:oasis:names:tc:SAML:2.0:protocol'><md:AssertionConsumerService Binding='urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST' Location='javascript:alert(true)' index='0' isDefault='true'/><md:AttributeConsumingService index='1' isDefault='true'><md:ServiceName xml:lang='en'>Required attributes</md:ServiceName><md:RequestedAttribute FriendlyName='Email address' Name='email' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Full name' Name='name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Given name' Name='first_name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Family name' Name='last_name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/></md:AttributeConsumingService></md:SPSSODescriptor></md:EntityDescriptor>
\ No newline at end of file

Fixed code: 	"fmt"
	"net/url"
// SOAPBindingV1 is the URN for the SOAP binding in SAML 1.0
const SOAPBindingV1 = "urn:oasis:names:tc:SAML:1.0:bindings:SOAP-binding"

func checkEndpointLocation(binding string, location string) (string, error) {
	// Within the SAML standard, the complex type EndpointType describes a
	// SAML protocol binding endpoint at which a SAML entity can be sent
	// protocol messages. In particular, the location of an endpoint type is
	// defined as follows in the Metadata for the OASIS Security Assertion
	// Markup Language (SAML) V2.0 - 2.2.2 Complex Type EndpointType:
	//
	//   Location [Required] A required URI attribute that specifies the
	//   location of the endpoint. The allowable syntax of this URI depends
	//   on the protocol binding.
	switch binding {
	case HTTPPostBinding,
		HTTPRedirectBinding,
		HTTPArtifactBinding,
		SOAPBinding,
		SOAPBindingV1:
		locationURL, err := url.Parse(location)
		if err != nil {
			return "", fmt.Errorf("invalid url %q: %w", location, err)
		}
		switch locationURL.Scheme {
		case "http", "https":
		// ok
		default:
			return "", fmt.Errorf("invalid url scheme %q for binding %q",
				locationURL.Scheme, binding)
		}
	default:
		// We don't know what form location should take, but the protocol
		// requires that we validate its syntax.
		//
		// In practice, lots of metadata contains random bindings, for example
		// "urn:mace:shibboleth:1.0:profiles:AuthnRequest" from our own test suite.
		//
		// We can't fail, but we also can't allow a location parameter whose syntax we
		// cannot verify. The least-bad course of action here is to set location to
		// and empty string, and hope the caller doesn't care need it.
		location = ""
	}

	return location, nil
}

// UnmarshalXML implements xml.Unmarshaler
func (m *Endpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
	type Alias Endpoint
	aux := &struct {
		*Alias
	}{
		Alias: (*Alias)(m),
	}
	if err := d.DecodeElement(aux, &start); err != nil {
		return err
	}

	var err error
	m.Location, err = checkEndpointLocation(m.Binding, m.Location)
	if err != nil {
		return err
	}
	if m.ResponseLocation != "" {
		m.ResponseLocation, err = checkEndpointLocation(m.Binding, m.ResponseLocation)
		if err != nil {
			return err
		}
	}

	return nil
}

// UnmarshalXML implements xml.Unmarshaler
func (m *IndexedEndpoint) UnmarshalXML(d *xml.Decoder, start xml.StartElement) error {
	type Alias IndexedEndpoint
	aux := &struct {
		*Alias
	}{
		Alias: (*Alias)(m),
	}
	if err := d.DecodeElement(aux, &start); err != nil {
		return err
	}

	var err error
	m.Location, err = checkEndpointLocation(m.Binding, m.Location)
	if err != nil {
		return err
	}
	if m.ResponseLocation != nil {
		responseLocation, err := checkEndpointLocation(m.Binding, *m.ResponseLocation)
		if err != nil {
			return err
		}
		if responseLocation != "" {
			m.ResponseLocation = &responseLocation
		} else {
			m.ResponseLocation = nil
		}
	}

	return nil
}


func TestMetadataValidatesUrlSchemeForProtocolBinding(t *testing.T) {
	buf := golden.Get(t, "TestMetadataValidatesUrlSchemeForProtocolBinding_metadata.xml")

	metadata := EntityDescriptor{}
	err := xml.Unmarshal(buf, &metadata)
	assert.Error(t, err, "invalid url scheme \"javascript\" for binding \"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST\"")
}
<?xml version='1.0' encoding='UTF-8'?><md:EntityDescriptor ID='_af805d1c-c2e3-444e-9cf5-efc664eeace6' entityID='https://dev.aa.kndr.org/users/auth/saml/metadata' validUntil='2001-02-03T04:05:06.789' cacheDuration='PT1H' xmlns:md='urn:oasis:names:tc:SAML:2.0:metadata' xmlns:saml='urn:oasis:names:tc:SAML:2.0:assertion'><md:SPSSODescriptor AuthnRequestsSigned='false' WantAssertionsSigned='false' protocolSupportEnumeration='urn:oasis:names:tc:SAML:2.0:protocol'><md:AssertionConsumerService Binding='urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST' Location='javascript:alert(true)' index='0' isDefault='true'/><md:AttributeConsumingService index='1' isDefault='true'><md:ServiceName xml:lang='en'>Required attributes</md:ServiceName><md:RequestedAttribute FriendlyName='Email address' Name='email' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Full name' Name='name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Given name' Name='first_name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/><md:RequestedAttribute FriendlyName='Family name' Name='last_name' NameFormat='urn:oasis:names:tc:SAML:2.0:attrname-format:basic'/></md:AttributeConsumingService></md:SPSSODescriptor></md:EntityDescriptor>