Fix panic on invalid client data

[zachhuff386]

Invalid data from the client will cause the server to panic from unchecked lengths

[6543]

I think you can add a testcase for this?

[james-d-elliott]

LGTM, the test is probably a good idea. Also see my suggestion. Per the spec the credentialIdLength must be less than or equal to 1023.

Also maybe we want to include it with minAuthDataLength/minAttestedAuthLength as something like maxCredentialIDLength. Also shouldn't these be consts?

[james-d-elliott]

Suggested change

	idLength := binary.BigEndian.Uint16(rawAuthData[53:55])
	if len(rawAuthData) < int(55+idLength) {
	return ErrBadRequest.WithDetails("Authenticator attestation data length too short")
	}
	idLength := binary.BigEndian.Uint16(rawAuthData[53:55])
	if len(rawAuthData) < int(55+idLength) {
	return ErrBadRequest.WithDetails("Authenticator attestation data length too short")
	}
	if idLength > 1023 {
	return ErrBadRequest.WithDetails("Authenticator attestation data credential id length too long")
	}

See https://w3c.github.io/webauthn/#attested-credential-data

[horgh]

I've encountered this a couple times too. Unfortunately I don't have the raw request data that caused it, but here's a stack trace:

server: net/http/server.go:3158: http: panic serving 172.16.0.105:46697: runtime error: slice bounds out of range [:45338] with capacity 181#012
goroutine 465640 [running]:#012net/http.(*conn).serve.func1()#012#011
    net/http/server.go:1802
+0xb9#012panic({0xd80b60, 0xc000674048})#012#011
    runtime/panic.go:1047
+0x266#012github.jparrowsec.cn/duo-labs/webauthn/protocol.(*AuthenticatorData).unmarshalAttestedData(0xc000027458, {0xc0006a0480, 0xc000094048, 0xc00015adc0})#012#011
    github.com/duo-labs/[email protected]/protocol/authenticator.go:210
+0x155#012github.jparrowsec.cn/duo-labs/webauthn/protocol.(*AuthenticatorData).Unmarshal(0xc000094048, {0xc0006a0480, 0xb5, 0xcb3680})#012#011
    github.com/duo-labs/[email protected]/protocol/authenticator.go:178
+0xf1#012github.jparrowsec.cn/duo-labs/webauthn/protocol.(*AuthenticatorAttestationResponse).Parse(0xc0002540b0)#012#011
    github.com/duo-labs/[email protected]/protocol/attestation.go:95
+0x245#012github.jparrowsec.cn/duo-labs/webauthn/protocol.ParseCredentialCreationResponseBody({0x1055bc0, 0xc0004b2330})#012#011
    github.com/duo-labs/[email protected]/protocol/credential.go:91

This is using go1.17.8 linux/amd64 and 4d1cf2d of this package.

The PR looks like it'd resolve it to me!

[james-d-elliott]

Yep that's precisely what this PR is looking to fix:

webauthn/protocol/authenticator.go

Line 210 in 4d1cf2d

a.AttData.CredentialID = rawAuthData[55 : 55+idLength]

[glacuesta-sa]

Firefox 98.0 is causing this error when Attestation=NONE in my case, can provide information about the test case if needed.

[horgh]

Would a new PR make sense at this point? I could give it a shot if so.

Info for the test case would be good probably in that case!

[6543]

well i guess so

[horgh]

I made #137, incorporating the comments here!