diff --git a/protocol/authenticator.go b/protocol/authenticator.go index 4b0ec51..d366053 100644 --- a/protocol/authenticator.go +++ b/protocol/authenticator.go @@ -8,7 +8,13 @@ import ( "github.com/duo-labs/webauthn/protocol/webauthncbor" ) -var minAuthDataLength = 37 +const ( + minAuthDataLength = 37 + minAttestedAuthLength = 55 + + // https://w3c.github.io/webauthn/#attested-credential-data + maxCredentialIDLength = 1023 +) // Authenticators respond to Relying Party requests by returning an object derived from the // AuthenticatorResponse interface. See ยง5.2. Authenticator Responses @@ -174,8 +180,11 @@ func (a *AuthenticatorData) Unmarshal(rawAuthData []byte) error { remaining := len(rawAuthData) - minAuthDataLength if a.Flags.HasAttestedCredentialData() { - if len(rawAuthData) > minAuthDataLength { - a.unmarshalAttestedData(rawAuthData) + if len(rawAuthData) > minAttestedAuthLength { + validError := a.unmarshalAttestedData(rawAuthData) + if validError != nil { + return validError + } attDataLen := len(a.AttData.AAGUID) + 2 + len(a.AttData.CredentialID) + len(a.AttData.CredentialPublicKey) remaining = remaining - attDataLen } else { @@ -204,11 +213,18 @@ func (a *AuthenticatorData) Unmarshal(rawAuthData []byte) error { } // If Attestation Data is present, unmarshall that into the appropriate public key structure -func (a *AuthenticatorData) unmarshalAttestedData(rawAuthData []byte) { +func (a *AuthenticatorData) unmarshalAttestedData(rawAuthData []byte) error { a.AttData.AAGUID = rawAuthData[37:53] idLength := binary.BigEndian.Uint16(rawAuthData[53:55]) + if len(rawAuthData) < int(55+idLength) { + return ErrBadRequest.WithDetails("Authenticator attestation data length too short") + } + if idLength > maxCredentialIDLength { + return ErrBadRequest.WithDetails("Authenticator attestation data credential id length too long") + } a.AttData.CredentialID = rawAuthData[55 : 55+idLength] a.AttData.CredentialPublicKey = unmarshalCredentialPublicKey(rawAuthData[55+idLength:]) + return nil } // Unmarshall the credential's Public Key into CBOR encoding diff --git a/protocol/authenticator_test.go b/protocol/authenticator_test.go index 135b40b..7d43ab6 100644 --- a/protocol/authenticator_test.go +++ b/protocol/authenticator_test.go @@ -2,6 +2,7 @@ package protocol import ( "encoding/base64" + "encoding/binary" "reflect" "testing" ) @@ -133,6 +134,14 @@ func TestAuthenticatorData_Unmarshal(t *testing.T) { noneAuthData, _ := base64.StdEncoding.DecodeString("pkLSG3xtVeHOI8U5mCjSx0m/am7y/gPMnhDN9O1TCItBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQMAxl6G32ykWaLrv/ouCs5HoGsvONqBtOb7ZmyMs8K8PccnwyyqPzWn/yZuyQmQBguvjYSvH6gDBlFG65quUDCSlAQIDJiABIVggyJGP+ra/u/eVjqN4OeYXUShRWxrEeC6Sb5/bZmJ9q8MiWCCHIkRdg5oRb1RHoFVYUpogcjlObCKFsV1ls1T+uUc6rA==") attAuthData, _ := base64.StdEncoding.DecodeString("lWkIjx7O4yMpVANdvRDXyuORMFonUbVZu4/Xy7IpvdRBAAAAAAAAAAAAAAAAAAAAAAAAAAAAQIniszxcGnhupdPFOHJIm6dscrWCC2h8xHicBMu91THD0kdOdB0QQtkaEn+6KfsfT1o3NmmFT8YfXrG734WfVSmlAQIDJiABIVggyoHHeiUw5aSbt8/GsL9zaqZGRzV26A4y3CnCGUhVXu4iWCBMnc8za5xgPzIygngAv9W+vZTMGJwwZcM4sjiqkcb/1g==") + attAuthDataIDExceedsBounds := make([]byte, len(attAuthData)) + copy(attAuthDataIDExceedsBounds, attAuthData) + binary.BigEndian.PutUint16(attAuthDataIDExceedsBounds[53:], 256) + + attAuthDataIDIsTooLarge := make([]byte, len(attAuthData)+2048) + copy(attAuthDataIDIsTooLarge, attAuthData) + binary.BigEndian.PutUint16(attAuthDataIDIsTooLarge[53:], maxCredentialIDLength+1) + tests := []struct { name string fields fields @@ -155,6 +164,22 @@ func TestAuthenticatorData_Unmarshal(t *testing.T) { }, false, }, + { + "Attestation data says the ID length is too large - extending beyond the buffer we're given", + fields{}, + args{ + attAuthDataIDExceedsBounds, + }, + true, + }, + { + "Attestation data says the ID length is too large - violates spec", + fields{}, + args{ + attAuthDataIDIsTooLarge, + }, + true, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -184,9 +209,10 @@ func TestAuthenticatorData_unmarshalAttestedData(t *testing.T) { rawAuthData []byte } tests := []struct { - name string - fields fields - args args + name string + fields fields + args args + wantErr bool }{ // TODO: Add test cases. } @@ -199,7 +225,9 @@ func TestAuthenticatorData_unmarshalAttestedData(t *testing.T) { AttData: tt.fields.AttData, ExtData: tt.fields.ExtData, } - a.unmarshalAttestedData(tt.args.rawAuthData) + if err := a.unmarshalAttestedData(tt.args.rawAuthData); (err != nil) != tt.wantErr { + t.Errorf("AuthenticatorData.unmarshalAttestedData() error = %v, wantErr %v", err, tt.wantErr) + } }) } }