Crash on nil-pointer dereference with malformed input

[stevenjohnstone]

I've been doing a bit of fuzzing of this package with go-fuzz. I captured a valid SAML response from Okta and then had go-fuzz mutate it. I have fuzzing implemented on this branch https://github.com/stevenjohnstone/gosaml2/tree/sjj/fuzzing.

Here's the panic in a simple test program:

package main

import (
	"crypto/x509"
	"encoding/base64"
	"encoding/xml"
	"time"

	saml2 "github.com/russellhaering/gosaml2"
	"github.com/russellhaering/gosaml2/types"
	dsig "github.com/russellhaering/goxmldsig"
)

const (
	timeString   = "2019-08-12T12:00:52.718Z"
	oktaMetadata = `<?xml version="1.0" encoding="UTF-8"?><md:EntityDescriptor entityID="http://www.okta.com/exk133onomIuOW98z357" xmlns:md="urn:oasis:names:tc:SAML:2.0:metadata"><md:IDPSSODescriptor WantAuthnRequestsSigned="false" protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol"><md:KeyDescriptor use="signing"><ds:KeyInfo xmlns:ds="http://www.w3.org/2000/09/xmldsig#"><ds:X509Data><ds:X509Certificate>MIIDpDCCAoygAwIBAgIGAWxzAwX1MA0GCSqGSIb3DQEBCwUAMIGSMQswCQYDVQQGEwJVUzETMBEG
A1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNU2FuIEZyYW5jaXNjbzENMAsGA1UECgwET2t0YTEU
MBIGA1UECwwLU1NPUHJvdmlkZXIxEzARBgNVBAMMCmRldi05MDUyNTExHDAaBgkqhkiG9w0BCQEW
DWluZm9Ab2t0YS5jb20wHhcNMTkwODA4MjA1MzMzWhcNMjkwODA4MjA1NDMzWjCBkjELMAkGA1UE
BhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExFjAUBgNVBAcMDVNhbiBGcmFuY2lzY28xDTALBgNV
BAoMBE9rdGExFDASBgNVBAsMC1NTT1Byb3ZpZGVyMRMwEQYDVQQDDApkZXYtOTA1MjUxMRwwGgYJ
KoZIhvcNAQkBFg1pbmZvQG9rdGEuY29tMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA
m+ZZF6aEG6ehLLIV6RPA+i1z6ss3HBG2bZD3efwKCDDXYUkp59AE7JsjVHMtpJPHhzHuScuHDMlu
HmkBQTW7j9XpnaRn8SfZXkwlCUHTo+HAC9lwbQxO4d4wnwgnm6FAjm1I/gbfFAobd8BR9pDxHuXE
MQ0DtQu/W3WbDUrz/bhSxPJAoVy2koQn9G0y3unm7eRwYWHeuW6GdPWV2szTtDS0c3qtUXVF5Ugg
iQYlwQu6xkfy4l8iGJL7ETa2BmJzwCFecMIct87SqNhYQwCBH54MBaHcaSsCKyimNvMY9B7RmC+H
4+awePPA1q3R/UQ3Pfom8mx6yDdKIWqlkG3MsQIDAQABMA0GCSqGSIb3DQEBCwUAA4IBAQAiURCZ
P4oJWcf1o5nm4yG15UH01g/S6Y4OUWMi6BFJy9fCrJ0h/2BZKi68SQ0uMAbdK6anxCzq3Rr5MSzW
OWPQ1Zljn3LGPsiTFdFca/GVRen5IYQ7Dr2Mvhtm+QVscEY9TDjtETbTAHEVEjwXmB21wtdIhizv
sQS7wz0A8LV+Atpbev45RiV6COmB6T6vJuFQ7ZsDZMSHZriTYiETTJvHBGd7PtbCxYNc6LRB2JDb
wlekRhVEjR0UhnM+nn2sqqbv7tDEPs63lZSDXCnR1PhscHrEuQ04rHI3OL0gCULVQFvJrj85IAZF
1QQuGUK8ozfOyFpQWAJUW71INnF/SLWv</ds:X509Certificate></ds:X509Data></ds:KeyInfo></md:KeyDescriptor><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified</md:NameIDFormat><md:NameIDFormat>urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress</md:NameIDFormat><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST" Location="https://dev-905251.okta.com/app/medev905251_test_1/exk133onomIuOW98z357/sso/saml"/><md:SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://dev-905251.okta.com/app/medev905251_test_1/exk133onomIuOW98z357/sso/saml"/></md:IDPSSODescriptor></md:EntityDescriptor>`

	badInput = `<saml2:Assertion ID="id1684056077776386493060641"IssueInstant="2019-08-12T12:00:52.718Z"Version="2.0"xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion"xmlns:xs="http://www.w3.org/2001/XMLSchema"><saml2:Issuer Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity"xmlns="">http://www.okta.com/exk133onomIuOW98z357</l><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/2001/04/xmldsig-more#rsa-sha256"/><ds:Reference URI="#id1684056077776386493060641"><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#"><ec:InclusiveNamespaces PrefixList="xs"xmlns:ec="http://www.w3.org/2001/10/xml-exc-c14n#"/></m></s><ds:DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><ds:DigestValue>dC1cm0pLLjIWZC6G2Pmf0JogmqHztp9W1euXPd/TUHo=</e></e></o><ds:SignatureValue>YRSCFLIkIgjbbYLyfCIc8jsP2MUJPjn+nYWRdlVIDdXtYXXxklYqdBXQsxDwNcsOAIGS75PeVGryml3oBkUDg/MfK7z/fFPLXX7c7xgh7/DBAFlSXbwlJQxuXQ5eZcGesgG6nYRwU1hpW+yN7C2ODN9KHi5TUdiEhvy8vdlFSfxdy4Mn68nG/UZBqmHHIZdRG2/Hpcs29YyaVVZUCZ0w22b7zsPuOXHuStOSTQ6isxI2R268+ZNKERYaNMCAGX4zNlT3mHBV0NnZkbO3wmlOfKksL+Qx7L64xFc3PaervxWuPqh2FoWpTCqFdliLdvUfFDszKXJKhO0bj1U0aSrdzg==</e><s><s><s></X></X></o></e><saml2:Subject xmlns=""><saml2:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified">steven.james.johnstone@gmail.com</l><saml2:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml2:SubjectConfirmationData InResponseTo="_40a419f5-5c1c-43d0-5834-5caf268a5f01"NotOnOrAfter="2019-08-12T12:05:52.718Z"Recipient="https://127.0.0.1/login"/></l></l><saml2:Conditions NotBefore="2019-08-12T11:55:52.718Z"NotOnOrAfter="2019-08-12T12:05:52.718Z"xmlns=""><saml2:AudienceRestriction><saml2:Audience>37a8eec1ce19687d132fe29051dca629d164e2c4958ba141d5f4133a33f0688f.jazznetworks.com</l></l></l><saml2:AuthnStatement AuthnInstant="2019-08-12T12:00:52.718Z"SessionIndex="_40a419f5-5c1c-43d0-5834-5caf268a5f01"xmlns=""><saml2:AuthnContext><saml2:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport</l></l></l><saml2:AttributeStatement xmlns=""><saml2:Attribute Name="FirstName"NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns=""xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:type="xs:string">Steven</l></l><saml2:Attribute Name="LastName"NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns=""xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:type="xs:string">Johnstone</l></l><saml2:Attribute Name="Email"NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified"><saml2:AttributeValue xmlns=""xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"xsi:type="xs:string">steven.james.johnstone@gmail.com`
)

func newServiceProvider() *saml2.SAMLServiceProvider {
	metadata := &types.EntityDescriptor{} // may need to support EntityDescriptors (plural)
	if err := xml.Unmarshal([]byte(oktaMetadata), metadata); err != nil {
		panic(err)
	}

	certStore := dsig.MemoryX509CertificateStore{
		Roots: []*x509.Certificate{},
	}

	for _, kd := range metadata.IDPSSODescriptor.KeyDescriptors {
		for _, xcert := range kd.KeyInfo.X509Data.X509Certificates {
			if xcert.Data == "" {
				panic("nope")
			}
			certData, err := base64.StdEncoding.DecodeString(xcert.Data)
			if err != nil {
				panic(err)
			}

			idpCert, err := x509.ParseCertificate(certData)
			if err != nil {
				panic(err)
			}

			certStore.Roots = append(certStore.Roots, idpCert)
		}
	}

	SSOs := metadata.IDPSSODescriptor.SingleSignOnServices

	tenantURI := "test.example.com"

	fakeTime, err := time.Parse(time.RFC3339, timeString)
	if err != nil {
		panic(err)
	}

	clock := dsig.NewFakeClockAt(fakeTime)

	return &saml2.SAMLServiceProvider{
		Clock:                       clock,
		IdentityProviderSSOURL:      SSOs[0].Location,
		IdentityProviderIssuer:      metadata.EntityID,
		ServiceProviderIssuer:       tenantURI,
		AssertionConsumerServiceURL: "https://127.0.0.1/login",
		SignAuthnRequests:           true,
		AudienceURI:                 tenantURI,
		IDPCertificateStore:         &certStore,
		ValidateEncryptionCert:      true,
	}
}

func main() {
	base64Input := base64.StdEncoding.EncodeToString([]byte(badInput))
	if _, err := newServiceProvider().RetrieveAssertionInfo(base64Input); err != nil {
		_ = err
	}
}

panic: runtime error: invalid memory address or nil pointer dereference                                                                                                                                                                                                                                                                                                                
[signal SIGSEGV: segmentation violation code=0x1 addr=0x18 pc=0x6727e6]                                                                                                                                                                                                                                                                                                                
                                                                                                                                                                                                                                                                                                                                                                                       
goroutine 1 [running]:                                                                                                                                                                                                                                                                                                                                                                 
github.com/russellhaering/gosaml2.(*SAMLServiceProvider).validateAssertionSignatures.func1(0xc000179f20, 0xc000182ea0, 0x9, 0x1)                                                                                                                                                                                                                                                       
    /goroot/go/src/github.com/russellhaering/gosaml2/decode_response.go:170 +0x56                                                                                                                                                                                                                                                                                                      
github.com/russellhaering/goxmldsig/etreeutils.NSFindIterateCtx.func1(0xc000179f20, 0xc000182ea0, 0xc000179f20, 0x0)                                                                                                                                                                                                                                                                   
    /goroot/go/src/github.com/russellhaering/goxmldsig/etreeutils/namespace.go:281 +0x137                                                                                                                                                                                                                                                                                              
github.com/russellhaering/goxmldsig/etreeutils.NSTraverse(0xc0000ccc60, 0xc000182ea0, 0xc000179ef0, 0x30, 0x6ebda0)                                                                                                                                                                                                                                                                    
    /goroot/go/src/github.com/russellhaering/goxmldsig/etreeutils/namespace.go:148 +0x6e                                                                                                                                                                                                                                                                                               
github.com/russellhaering/goxmldsig/etreeutils.NSFindIterateCtx(0xc0000ccc60, 0xc000182ea0, 0x71c4db, 0x25, 0x712a6e, 0x9, 0xc000179ec0, 0x7152bf, 0x12)                                                                                                                                                                                                                               
    /goroot/go/src/github.com/russellhaering/goxmldsig/etreeutils/namespace.go:268 +0xa4                                                                                                                                                                                                                                                                                               
github.com/russellhaering/goxmldsig/etreeutils.NSFindIterate(...)                                                                                                                                                                                                                                                                                                                      
    /goroot/go/src/github.com/russellhaering/goxmldsig/etreeutils/namespace.go:257                                                                                                                                                                                                                                                                                                     
github.com/russellhaering/gosaml2.(*SAMLServiceProvider).validateAssertionSignatures(0xc000166000, 0xc000182ea0, 0x0, 0x0)                                                                                                                                                                                                                                                             
    /goroot/go/src/github.com/russellhaering/gosaml2/decode_response.go:200 +0xed                                                                                                                                                                                                                                                                                                      
github.com/russellhaering/gosaml2.(*SAMLServiceProvider).ValidateEncodedResponse(0xc000166000, 0xc000145000, 0xf80, 0xc000013510, 0xc000160000, 0xc0000d8b40)                                                                                                                                                                                                                          
    /goroot/go/src/github.com/russellhaering/gosaml2/decode_response.go:248 +0x30e                                                                                                                                                                                                                                                                                                     
github.com/russellhaering/gosaml2.(*SAMLServiceProvider).RetrieveAssertionInfo(0xc000166000, 0xc000145000, 0xf80, 0xba0, 0xc000145000, 0xf80)                                                                                                                                                                                                                                          
    /goroot/go/src/github.com/russellhaering/gosaml2/retrieve_assertion.go:40 +0x9b                                                                                                                                                                                                                                                                                                    
main.main()                                                                                                                                                                                                                                                                                                                                                                            
    /goroot/go/src/github.com/russellhaering/gosaml2/crasher/main.go:92 +0xaa                                                                                                                                                                                                                                                                                                          
exit status 2

Appears to be unverified assertions can have nil parents. Appears to be fixed with this patch:

index a40e4b1..0804962 100644
--- a/decode_response.go
+++ b/decode_response.go
@@ -6,6 +6,7 @@ import (
        "crypto/tls"
        "crypto/x509"
        "encoding/base64"
+       "errors"
        "fmt"
        "io/ioutil"
 
@@ -166,7 +167,11 @@ func (sp *SAMLServiceProvider) validateAssertionSignatures(el *etree.Element) er
        signedAssertions := 0
        unsignedAssertions := 0
        validateAssertion := func(ctx etreeutils.NSContext, unverifiedAssertion *etree.Element) error {
-               if unverifiedAssertion.Parent() != el {
+               parent := unverifiedAssertion.Parent()
+               if parent == nil {
+                       return errors.New("parent is nil")
+               }
+               if parent != el {
                        return fmt.Errorf("found assertion with unexpected parent element: %s", unverifiedAssertion.Parent().Tag)
                }
 

[stevenjohnstone]

Note that the example above needs to built with go.mod along the lines of

module example.com/foo

go 1.15

require (
	github.com/russellhaering/gosaml2 v0.6.0
)

Without using modules, the latest version of etree is pulled in which rejects the xml due to unbalanced xml tags i.e. with the following go.mod

module example.com/foo

go 1.15

require (
        github.com/beevik/etree v1.1.1-0.20200718192613-4a2f8b9d084c
	github.com/russellhaering/gosaml2 v0.6.0
)

the crasher no longer works.

[mfridman]

Hey @russellhaering what are your thoughts on patching this up and releasing a v0.6.1 ?

[miguelff]

Are there any plans to roll out 0.6.1 so we can upgrade our dependencies?

[ddillard]

This issue has been around for over a year now, a patch was proposed, yet nothing has been done to address the issue. So, I have to ask, is this project effectively dead?

[lukyer]

@russellhaering could you please have a look? This vulnerability drives all devsecops tools crazy :) Thanks!

[mfridman]

@lukyer To mitigate this we pinned the latest (untagged) commit on the main branch.

See #86 (comment)

[lukyer]

@mfridman ah cool, thanks!

go get -u github.com/russellhaering/gosaml2@v0.6.1-0.20210916051624-757d23f1bc28

to the rescue :)