Fix panic on invalid client data

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

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)
+			}
 		})
 	}
 }