Skip to content

Commit

Permalink
fix(webauthn): potential panic in parse fido public key (#39)
Browse files Browse the repository at this point in the history
This fixes a potential panic that can occur when calling ParseFIDOPublicKey because elliptic.Unamrshall can return two nil values if the
  • Loading branch information
james-d-elliott authored Jun 24, 2022
1 parent fc740cf commit 3551cfa
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ There are several differences between the upstream library and this one. We will
* The following misc fixes have been merged:
* Ensuring the credential ID length is not too long in [b3b93ac](https://github.com/go-webauthn/webauthn/commit/b3b93ac3770a26a92adbcd4b527bbb391127931b) (v0.2.x) and [35287ea](https://github.com/go-webauthn/webauthn/commit/35287ea54b50b1f553f3cc0f0f5527039f375e2c) (v0.1.x).
* Ensuring errors are effectively checked, ineffectual checks are not done, and general linting fixes in [90be0fe](https://github.com/go-webauthn/webauthn/commit/90be0fe276222bd574cf19856081979789ce9fca).
* A potential nil pointer error in ParseFIDOPublicKey.

## Status

Expand Down
21 changes: 10 additions & 11 deletions protocol/webauthncose/webauthncose.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,20 +158,18 @@ func HasherFromCOSEAlg(coseAlg COSEAlgorithmIdentifier) func() hash.Hash {
}

// Figure out what kind of COSE material was provided and create the data for the new key
func ParsePublicKey(keyBytes []byte) (interface{}, error) {
func ParsePublicKey(keyBytes []byte) (k interface{}, err error) {
pk := PublicKeyData{}

err := webauthncbor.Unmarshal(keyBytes, &pk)
if err != nil {
if err = webauthncbor.Unmarshal(keyBytes, &pk); err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall Public Key data: %v", err))
}

switch COSEKeyType(pk.KeyType) {
case OctetKey:
var o OKPPublicKeyData

err := webauthncbor.Unmarshal(keyBytes, &o)
if err != nil {
if err = webauthncbor.Unmarshal(keyBytes, &o); err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall OK Public Key data: %v", err))
}

Expand All @@ -181,8 +179,7 @@ func ParsePublicKey(keyBytes []byte) (interface{}, error) {
case EllipticKey:
var e EC2PublicKeyData

err := webauthncbor.Unmarshal(keyBytes, &e)
if err != nil {
if err = webauthncbor.Unmarshal(keyBytes, &e); err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall EC2 Public Key data: %v", err))
}

Expand All @@ -192,8 +189,7 @@ func ParsePublicKey(keyBytes []byte) (interface{}, error) {
case RSAKey:
var r RSAPublicKeyData

err := webauthncbor.Unmarshal(keyBytes, &r)
if err != nil {
if err = webauthncbor.Unmarshal(keyBytes, &r); err != nil {
return nil, ErrUnsupportedKey.WithDetails(fmt.Sprintf("Could not unmarshall RSA Public Key data: %v", err))
}

Expand All @@ -205,9 +201,13 @@ func ParsePublicKey(keyBytes []byte) (interface{}, error) {
}
}

func ParseFIDOPublicKey(keyBytes []byte) (EC2PublicKeyData, error) {
func ParseFIDOPublicKey(keyBytes []byte) (interface{}, error) {
x, y := elliptic.Unmarshal(elliptic.P256(), keyBytes)

if x == nil || y == nil {
return nil, ErrUnsupportedKey.WithDetails("Could not unmarshall EC2 Public Key data")
}

return EC2PublicKeyData{
PublicKeyData: PublicKeyData{
Algorithm: int64(AlgES256),
Expand Down Expand Up @@ -262,7 +262,6 @@ const (
)

func VerifySignature(key interface{}, data []byte, sig []byte) (bool, error) {

switch k := key.(type) {
case OKPPublicKeyData:
return k.Verify(data, sig)
Expand Down
2 changes: 1 addition & 1 deletion webauthn/registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (

type RegistrationOption func(*protocol.PublicKeyCredentialCreationOptions)

// Generate a new set of registration data to be sent to the client and authenticator.
// BeginRegistration generates a new set of registration data to be sent to the client and authenticator.
func (webauthn *WebAuthn) BeginRegistration(user User, opts ...RegistrationOption) (*protocol.CredentialCreation, *SessionData, error) {
challenge, err := protocol.CreateChallenge()
if err != nil {
Expand Down

0 comments on commit 3551cfa

Please sign in to comment.