Skip to content

Commit

Permalink
Merge pull request from GHSA-xhqq-x44f-9fgg
Browse files Browse the repository at this point in the history 
Implement xml-roundtrip-validator
  • Loading branch information
@russellhaering
russellhaering authored Dec 14, 2020
2 parents 7f4c424 + 0f0fb74 commit 42606da
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 0 deletions.
9 changes: 9 additions & 0 deletions decode_response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/russellhaering/gosaml2/types"
dsig "github.com/russellhaering/goxmldsig"
"github.com/russellhaering/goxmldsig/etreeutils"
rtvalidator "github.com/mattermost/xml-roundtrip-validator"
)

func (sp *SAMLServiceProvider) validationContext() *dsig.ValidationContext {
Expand Down Expand Up @@ -355,9 +356,11 @@ func maybeDeflate(data []byte, decoder func([]byte) error) error {
// parseResponse is a helper function that was refactored out so that the XML parsing behavior can be isolated and unit tested
func parseResponse(xml []byte) (*etree.Document, *etree.Element, error) {
var doc *etree.Document
var rawXML []byte

err := maybeDeflate(xml, func(xml []byte) error {
doc = etree.NewDocument()
rawXML = xml
return doc.ReadFromBytes(xml)
})
if err != nil {
Expand All @@ -369,6 +372,12 @@ func parseResponse(xml []byte) (*etree.Document, *etree.Element, error) {
return nil, nil, fmt.Errorf("unable to parse response")
}

// Examine the response for attempts to exploit weaknesses in Go's encoding/xml
err = rtvalidator.Validate(bytes.NewReader(rawXML))
if err != nil {
return nil, nil, err
}

return doc, el, nil
}

Expand Down
10 changes: 10 additions & 0 deletions decode_response_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,13 @@ func TestCompressedResponse(t *testing.T) {
_, err = sp.RetrieveAssertionInfo(string(bs))
require.NoError(t, err, "Assertion info should be retrieved with no error")
}

func TestDecodeColonsInLocalNames(t *testing.T) {
_, _, err := parseResponse([]byte(`<x::Root/>`))
require.Error(t, err)
}

func TestDecodeDoubleColonInjectionAttackResponse(t *testing.T) {
_, _, err := parseResponse([]byte(doubleColonAssertionInjectionAttackResponse))
require.Error(t, err)
}
1 change: 1 addition & 0 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ go 1.13
require (
github.com/beevik/etree v1.1.0
github.com/jonboulle/clockwork v0.2.0
github.com/mattermost/xml-roundtrip-validator v0.0.0-20201208211235-fe770d50d911
github.com/russellhaering/goxmldsig v1.1.0
github.com/stretchr/testify v1.6.1
)
2 changes: 2 additions & 0 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/jonboulle/clockwork v0.2.0 h1:J2SLSdy7HgElq8ekSl2Mxh6vrRNFxqbXGenYH2I02Vs=
github.com/jonboulle/clockwork v0.2.0/go.mod h1:Pkfl5aHPm1nk2H9h0bjmnJD/BcgbGXUBGnn1kMkgxc8=
github.com/mattermost/xml-roundtrip-validator v0.0.0-20201208211235-fe770d50d911 h1:erppMjjp69Rertg1zlgRbLJH1u+eCmRPxKjMZ5I8/Ro=
github.com/mattermost/xml-roundtrip-validator v0.0.0-20201208211235-fe770d50d911/go.mod h1:qccnGMcpgwcNaBnxqpJpWWUiPNr5H3O8eDgGV9gT5To=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/russellhaering/goxmldsig v1.1.0 h1:lK/zeJie2sqG52ZAlPNn1oBBqsIsEKypUUBGpYYF6lk=
Expand Down
10 changes: 10 additions & 0 deletions test_constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,3 +406,13 @@ DJpRaioUTd2lGh4TLUxAxCxtUk/pascL+3Nn936LFmUCLxaxnbeGzPOXAhscCtU1H0nFsXRnKx5a
cPXYSKFZZZktieSkww2Oi8dg2DYaQhGQMSFMVqgVfwEu4bvCRBvdSiNXdWGCZQmFVzBZZ/9rOLzP
pvTFTPnpkavJm81FLlUhiE/oFgKlCDLWDknSpXAI0uZGERcwPca6xvIMh86LjQKjbVci9FYDStXC
qRnqQ+TccSu/B6uONFsDEngGcXSKfB+a</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><saml2:Subject xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><saml2:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">[email protected]<!---->.evil.com</saml2:NameID><saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml2:SubjectConfirmationData InResponseTo="_da213df8-ef95-41d0-b9bf-71d271735cd7" NotOnOrAfter="2116-03-28T16:43:18.565Z" Recipient="http://localhost:8080/v1/_saml_callback"/></saml2:SubjectConfirmation></saml2:Subject><saml2:Conditions NotBefore="2016-03-28T16:33:18.565Z" NotOnOrAfter="2116-03-28T16:43:18.565Z" xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><saml2:AudienceRestriction><saml2:Audience>123</saml2:Audience></saml2:AudienceRestriction></saml2:Conditions><saml2:AuthnStatement AuthnInstant="2016-03-28T16:38:18.565Z" SessionIndex="_da213df8-ef95-41d0-b9bf-71d271735cd7" xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><saml2:AuthnContext><saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml2:AuthnContextClassRef></saml2:AuthnContext></saml2:AuthnStatement><saml2:AttributeStatement xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"><saml2:Attribute Name="FirstName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Phoebe</saml2:AttributeValue></saml2:Attribute><saml2:Attribute Name="LastName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Simon</saml2:AttributeValue></saml2:Attribute><saml2:Attribute Name="Email" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">[email protected]</saml2:AttributeValue></saml2:Attribute><saml2:Attribute Name="Login" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">phoebesimon</saml2:AttributeValue></saml2:Attribute></saml2:AttributeStatement></saml2:Assertion></saml2p:Response>`


const doubleColonAssertionInjectionAttackResponse = `
<samlp:Response xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" ID="R060bff490336a09324ed664f6e8b03fa12dc1994" Version="2.0" IssueInstant="2017-03-08T07:53:39Z" Destination="http://884d40bf.ngrok.io/api/sso/saml2/acs/58af624473d4f375b8e70d81">
<saml:Issuer>https://app.onelogin.com/saml/metadata/634027</saml:Issuer>
<samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status>
<::Assertion xmlns="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Version="2.0" ID="x" IssueInstant="2017-03-08T07:53:39Z"><saml:Issuer>https://app.onelogin.com/saml/metadata/634027</saml:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/><ds:Reference URI="#x"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>gd5V090n/m4JRrtpo5WgrwPyyy0=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue></ds:SignatureValue><ds:KeyInfo><ds:X509Data><ds:X509Certificate></ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">[email protected]</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotOnOrAfter="2017-03-08T07:56:39Z" Recipient="http://884d40bf.ngrok.io/api/sso/saml2/acs/58af624473d4f375b8e70d81"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2017-03-08T07:50:39Z" NotOnOrAfter="2017-03-08T07:56:39Z"><saml:AudienceRestriction><saml:Audience>{audience}</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AuthnStatement AuthnInstant="2017-03-08T07:53:38Z" SessionNotOnOrAfter="2017-03-09T07:53:39Z" SessionIndex="_d5fe4830-e601-0134-4e06-0af7aa36d0f9"><saml:AuthnContext><saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></::Assertion>
<saml:Assertion xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" Version="2.0" ID="pfx63cf6dc4-c309-ff5e-6049-84c34f0c0061" IssueInstant="2017-03-08T07:53:39Z"><saml:Issuer>https://app.onelogin.com/saml/metadata/634027</saml:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/><ds:Reference URI="#pfx63cf6dc4-c309-ff5e-6049-84c34f0c0061"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>gd5V090n/m4JRrtpo5WgrwPyyy0=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>SLzvdNM+1R1+3XsXpC+/RIvb5L4Lhy7Eb7caPG2CLMPYhzbKLAwIiT7/0fEMO/xL7rdIgEShbcU9iu5PX4hGYBhirsFIZvdHytns5+JKHnlVBmHm4TsSU1z+dGMXBa//L0KFSrvdgBUpsr5vs50SuYnnVp61VN+zCLMqO221CQfP95QyMcSQ+fiyq4GOmWLwQy1m1+NV3U8zlapp6FIH5stW/dp4OqpRdafV96rVwmmR4yeUw7VAzbJuMrPgkXO9nUbHeMUTgQxkQ4ThzG5jt6fT+Ro1NOYS4zpVtzqlQwGzqWxQVRLEqXIf500/Qi0NuFQOW42ZAUiXDgdLENTVGA==</ds:SignatureValue><ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIEJjCCAw6gAwIBAgIUOHrykO4ce1TbjvGgXXVVnR4NsqMwDQYJKoZIhvcNAQEFBQAwXTELMAkGA1UEBhMCVVMxFTATBgNVBAoMDExhdW5jaERhcmtseTEVMBMGA1UECwwMT25lTG9naW4gSWRQMSAwHgYDVQQDDBdPbmVMb2dpbiBBY2NvdW50IDEwMjEyNzAeFw0xNzAzMDYwMjQ2NTNaFw0yMjAzMDcwMjQ2NTNaMF0xCzAJBgNVBAYTAlVTMRUwEwYDVQQKDAxMYXVuY2hEYXJrbHkxFTATBgNVBAsMDE9uZUxvZ2luIElkUDEgMB4GA1UEAwwXT25lTG9naW4gQWNjb3VudCAxMDIxMjcwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCaJ02AnJe5vq+zzkmrIHhRy8V/UxJogbJGEJW6nqrEmO7Q4sXO7dLIKxGccCEz0KAavGKWzSX9uhVvKpazpD4bW80wPQIgFxN3CjiA3qlYIfhhh4emSZo2AnaTuG4BPVGFNPx0jxXGAhh/3xkpIsqARJFPB6njT2+MwFctm3fockx3Yp4e1xoUD8qQR0f/8oq1LjrYd2Vlckmmw7qrzSqS8POHW/I1jx9Y/vAjTPWDKXmbmLcTe3188PDrthSyoBuaAGBRVTP9WTuYMh4kGvmfX6sNvIDGejUcUCq6IObRr4xLSZiGy5uoyqsQc9agAhQm+26Gpq0R3NSvN91JdbZHAgMBAAGjgd0wgdowDAYDVR0TAQH/BAIwADAdBgNVHQ4EFgQUnbxBsHgNVq3OSXEuG5EkR0Jd1UswgZoGA1UdIwSBkjCBj4AUnbxBsHgNVq3OSXEuG5EkR0Jd1UuhYaRfMF0xCzAJBgNVBAYTAlVTMRUwEwYDVQQKDAxMYXVuY2hEYXJrbHkxFTATBgNVBAsMDE9uZUxvZ2luIElkUDEgMB4GA1UEAwwXT25lTG9naW4gQWNjb3VudCAxMDIxMjeCFDh68pDuHHtU247xoF11VZ0eDbKjMA4GA1UdDwEB/wQEAwIHgDANBgkqhkiG9w0BAQUFAAOCAQEAL/6j2qpMCrnolwKT7mfPEpA6btbtl0R0t6zSwYUVU9T3PK0/P3LKXvbjSySov0E4R9d5qlOcyj5CbYiuqAO2aON3xy82s0dN3FHRiO6kcjoRPwVIIF0S8x7tpzcPKa42zSPfBqMRw4ezUEzTijFriepkSWST1Btr3QeK2Cxhr0fC1xmw/YK82BV0/oVRslGL27ro+v3/dNY0A0r32Xe2+THomrY/YaZaDCPCjHo8dlxrX3D/mPfoiiKSkm2mGagQXT0giTHVo3oIq+u+KdrBcQn65EBcjfFKDIeFCdiVmO0xPl9mmWskVRLy2/wpuDIp6hnAphl9lj5DY48eBsrEXQ==</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><saml:Subject><saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">[email protected]</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotOnOrAfter="2017-03-08T07:56:39Z" Recipient="http://884d40bf.ngrok.io/api/sso/saml2/acs/58af624473d4f375b8e70d81"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2017-03-08T07:50:39Z" NotOnOrAfter="2017-03-08T07:56:39Z"><saml:AudienceRestriction><saml:Audience>{audience}</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AuthnStatement AuthnInstant="2017-03-08T07:53:38Z" SessionNotOnOrAfter="2017-03-09T07:53:39Z" SessionIndex="_d5fe4830-e601-0134-4e06-0af7aa36d0f9"><saml:AuthnContext><saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement></saml:Assertion>
</samlp:Response>
`

0 comments on commit 42606da

Please sign in to comment.