Repository: duo-labs/py_webauthn

Issue:
cbor2 exceptions are not caught
If an invalid payload is sent to a server that uses `verify_registration_response` 

Example:

```diff
index 7dab1df..b711a31 100644
--- tests/test_verify_registration_response.py
+++ tests/test_verify_registration_response.py
@@ -21,7 +21,7 @@ class TestVerifyRegistrationResponse(TestCase):
             "id": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
             "rawId": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
             "response": {
-                "attestationObject": "o2NmbXRkbm9uZWdhdHRTdG10oGhhdXRoRGF0YVjESZYN5YgOjGh0NBcPZHZgW4_krrmihjLHmVzzuoMdl2NFAAAAFwAAAAAAAAAAAAAAAAAAAAAAQPctcQPE5oNRRJk_nO_371mf7qE7qIodzr0eOf6ACvnMB1oQG165dqutoi1U44shGezu5_gkTjmOPeJO0N8a7P-lAQIDJiABIVggSFbUJF-42Ug3pdM8rDRFu_N5oiVEysPDB6n66r_7dZAiWCDUVnB39FlGypL-qAoIO9xWHtJygo2jfDmHl-_eKFRLDA",
+                "attestationObject": "",
                 "clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiVHdON240V1R5R0tMYzRaWS1xR3NGcUtuSE00bmdscXN5VjBJQ0psTjJUTzlYaVJ5RnRya2FEd1V2c3FsLWdrTEpYUDZmbkYxTWxyWjUzTW00UjdDdnciLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUwMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2V9"
             },
             "type": "public-key",
```

Error when running test:

```
======================================================================
ERROR: test_verifies_none_attestation_response (tests.test_verify_registration_response.TestVerifyRegistrationResponse.test_verifies_none_attestation_response)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/iiqbal/code/upstream/py_webauthn/tests/test_verify_registration_response.py", line 42, in test_verifies_none_attestation_response
    verification = verify_registration_response(
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/iiqbal/code/upstream/py_webauthn/webauthn/registration/verify_registration_response.py", line 141, in verify_registration_response
    attestation_object = parse_attestation_object(response.attestation_object)
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/iiqbal/code/upstream/py_webauthn/webauthn/helpers/parse_attestation_object.py", line 13, in parse_attestation_object
    attestation_dict = cbor2.loads(val)
                       ^^^^^^^^^^^^^^^^
_cbor2.CBORDecodeEOF: premature end of stream (expected to read 1 bytes, got 0 instead)

----------------------------------------------------------------------
```

Thoughts on adding a cbor parsing exception similar to the client data json exception?


Discussion:

Patch:
diff --git a/webauthn/helpers/__init__.py b/webauthn/helpers/__init__.py
index ff3fdd9..5a2ded0 100644
--- a/webauthn/helpers/__init__.py
+++ b/webauthn/helpers/__init__.py
@@ -12,6 +12,7 @@ from .parse_attestation_object import parse_attestation_object
 from .parse_authentication_credential_json import parse_authentication_credential_json
 from .parse_authenticator_data import parse_authenticator_data
 from .parse_backup_flags import parse_backup_flags
+from .parse_cbor import parse_cbor
 from .parse_client_data_json import parse_client_data_json
 from .parse_registration_credential_json import parse_registration_credential_json
 from .validate_certificate_chain import validate_certificate_chain
@@ -33,6 +34,7 @@ __all__ = [
     "parse_authenticator_data",
     "parse_authentication_credential_json",
     "parse_backup_flags",
+    "parse_cbor",
     "parse_client_data_json",
     "parse_registration_credential_json",
     "validate_certificate_chain",
diff --git a/webauthn/helpers/exceptions.py b/webauthn/helpers/exceptions.py
index b26fc9c..1f58ded 100644
--- a/webauthn/helpers/exceptions.py
+++ b/webauthn/helpers/exceptions.py
@@ -52,3 +52,7 @@ class InvalidCertificateChain(Exception):
 
 class InvalidBackupFlags(Exception):
     pass
+
+
+class InvalidCBORData(Exception):
+    pass
diff --git a/webauthn/helpers/parse_authenticator_data.py b/webauthn/helpers/parse_authenticator_data.py
index 5792def..7d58de1 100644
--- a/webauthn/helpers/parse_authenticator_data.py
+++ b/webauthn/helpers/parse_authenticator_data.py
@@ -2,6 +2,7 @@ import cbor2
 
 from .exceptions import InvalidAuthenticatorDataStructure
 from .structs import AttestedCredentialData, AuthenticatorData, AuthenticatorDataFlags
+from .parse_cbor import parse_cbor
 
 
 def parse_authenticator_data(val: bytes) -> AuthenticatorData:
@@ -75,7 +76,7 @@ def parse_authenticator_data(val: bytes) -> AuthenticatorData:
             val = bytes(_val)
 
         # Load the next CBOR-encoded value
-        credential_public_key = cbor2.loads(val[pointer:])
+        credential_public_key = parse_cbor(val[pointer:])
         credential_public_key_bytes = cbor2.dumps(credential_public_key)
         pointer += len(credential_public_key_bytes)
 
@@ -87,7 +88,7 @@ def parse_authenticator_data(val: bytes) -> AuthenticatorData:
         authenticator_data.attested_credential_data = attested_cred_data
 
     if flags.ed is True:
-        extension_object = cbor2.loads(val[pointer:])
+        extension_object = parse_cbor(val[pointer:])
         extension_bytes = cbor2.dumps(extension_object)
         pointer += len(extension_bytes)
         authenticator_data.extensions = extension_bytes
diff --git a/webauthn/helpers/parse_cbor.py b/webauthn/helpers/parse_cbor.py
new file mode 100644
index 0000000..a98ae49
--- /dev/null
+++ b/webauthn/helpers/parse_cbor.py
@@ -0,0 +1,22 @@
+from typing import Any
+
+import cbor2
+
+from .exceptions import InvalidCBORData
+
+
+def parse_cbor(data: bytes) -> Any:
+    """
+    Attempt to decode CBOR-encoded data.
+
+    Raises:
+        `helpers.exceptions.InvalidCBORData` if data cannot be decoded
+    """
+    try:
+        to_return = cbor2.loads(data)
+    except Exception as exc:
+        raise InvalidCBORData(
+            "Could not decode CBOR data"
+        ) from exc
+
+    return to_return
diff --git a/webauthn/registration/formats/android_key.py b/webauthn/registration/formats/android_key.py
index 10906e9..9c441d1 100644
--- a/webauthn/registration/formats/android_key.py
+++ b/webauthn/registration/formats/android_key.py
@@ -1,7 +1,6 @@
 import hashlib
 from typing import List
 
-import cbor2
 from cryptography import x509
 from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.backends import default_backend
@@ -16,6 +15,7 @@ from cryptography.x509 import (
 from webauthn.helpers import (
     decode_credential_public_key,
     decoded_public_key_to_cryptography,
+    parse_cbor,
     validate_certificate_chain,
     verify_signature,
 )
@@ -79,7 +79,7 @@ def verify_android_key(
         raise InvalidRegistrationResponse(f"{err} (Android Key)")
 
     # Extract attStmt bytes from attestation_object
-    attestation_dict = cbor2.loads(attestation_object)
+    attestation_dict = parse_cbor(attestation_object)
     authenticator_data_bytes = attestation_dict["authData"]
 
     # Generate a hash of client_data_json
diff --git a/webauthn/registration/formats/android_safetynet.py b/webauthn/registration/formats/android_safetynet.py
index 5acf807..8da9f47 100644
--- a/webauthn/registration/formats/android_safetynet.py
+++ b/webauthn/registration/formats/android_safetynet.py
@@ -2,7 +2,6 @@ import base64
 import hashlib
 from typing import List
 
-import cbor2
 from cryptography import x509
 from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.backends import default_backend
@@ -11,6 +10,7 @@ from cryptography.x509.oid import NameOID
 from webauthn.helpers.cose import COSEAlgorithmIdentifier
 from webauthn.helpers import (
     base64url_to_bytes,
+    parse_cbor,
     validate_certificate_chain,
     verify_safetynet_timestamp,
     verify_signature,
@@ -101,7 +101,7 @@ def verify_android_safetynet(
     # clientDataHash.
 
     # Extract attStmt bytes from attestation_object
-    attestation_dict = cbor2.loads(attestation_object)
+    attestation_dict = parse_cbor(attestation_object)
     authenticator_data_bytes = attestation_dict["authData"]
 
     # Generate a hash of client_data_json
diff --git a/webauthn/registration/formats/apple.py b/webauthn/registration/formats/apple.py
index b1daa1a..63cf664 100644
--- a/webauthn/registration/formats/apple.py
+++ b/webauthn/registration/formats/apple.py
@@ -15,6 +15,7 @@ from cryptography.x509 import (
 from webauthn.helpers import (
     decode_credential_public_key,
     decoded_public_key_to_cryptography,
+    parse_cbor,
     validate_certificate_chain,
 )
 from webauthn.helpers.exceptions import (
@@ -55,7 +56,7 @@ def verify_apple(
         raise InvalidRegistrationResponse(f"{err} (Apple)")
 
     # Concatenate authenticatorData and clientDataHash to form nonceToHash.
-    attestation_dict = cbor2.loads(attestation_object)
+    attestation_dict = parse_cbor(attestation_object)
     authenticator_data_bytes = attestation_dict["authData"]
 
     client_data_hash = hashlib.sha256()
diff --git a/webauthn/registration/formats/packed.py b/webauthn/registration/formats/packed.py
index 39119ca..4c9849b 100644
--- a/webauthn/registration/formats/packed.py
+++ b/webauthn/registration/formats/packed.py
@@ -1,7 +1,6 @@
 import hashlib
 from typing import List
 
-import cbor2
 from cryptography import x509
 from cryptography.exceptions import InvalidSignature
 from cryptography.hazmat.backends import default_backend
@@ -9,6 +8,7 @@ from cryptography.hazmat.backends import default_backend
 from webauthn.helpers import (
     decode_credential_public_key,
     decoded_public_key_to_cryptography,
+    parse_cbor,
     validate_certificate_chain,
     verify_signature,
 )
@@ -42,7 +42,7 @@ def verify_packed(
         )
 
     # Extract attStmt bytes from attestation_object
-    attestation_dict = cbor2.loads(attestation_object)
+    attestation_dict = parse_cbor(attestation_object)
     authenticator_data_bytes = attestation_dict["authData"]
 
     # Generate a hash of client_data_json
diff --git a/webauthn/registration/formats/tpm.py b/webauthn/registration/formats/tpm.py
index a1d775b..84a3d59 100644
--- a/webauthn/registration/formats/tpm.py
+++ b/webauthn/registration/formats/tpm.py
@@ -18,6 +18,7 @@ from cryptography.x509.oid import ExtensionOID
 from webauthn.helpers import (
     decode_credential_public_key,
     hash_by_alg,
+    parse_cbor,
     validate_certificate_chain,
     verify_signature,
 )
@@ -153,7 +154,7 @@ def verify_tpm(
         )
 
     # Concatenate authenticatorData and clientDataHash to form attToBeSigned.
-    attestation_dict = cbor2.loads(attestation_object)
+    attestation_dict = parse_cbor(attestation_object)
     authenticator_data_bytes: bytes = attestation_dict["authData"]
     client_data_hash = hash_by_alg(client_data_json)
     att_to_be_signed = b"".join(


Test patch:
diff --git a/tests/test_verify_registration_response.py b/tests/test_verify_registration_response.py
index e8949cf..c693c90 100644
--- a/tests/test_verify_registration_response.py
+++ b/tests/test_verify_registration_response.py
@@ -4,7 +4,7 @@ from unittest import TestCase
 import cbor2
 from pydantic import ValidationError
 from webauthn.helpers import base64url_to_bytes, bytes_to_base64url, parse_registration_credential_json
-from webauthn.helpers.exceptions import InvalidRegistrationResponse
+from webauthn.helpers.exceptions import InvalidRegistrationResponse, InvalidCBORData
 from webauthn.helpers.known_root_certs import globalsign_r2
 from webauthn.helpers.structs import (
     AttestationFormat,
@@ -251,3 +251,32 @@ class TestVerifyRegistrationResponse(TestCase):
         )
 
         assert verification.fmt == AttestationFormat.NONE
+
+    def test_raises_useful_error_on_bad_attestation_object(self) -> None:
+        credential = {
+            "id": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
+            "rawId": "9y1xA8Tmg1FEmT-c7_fvWZ_uoTuoih3OvR45_oAK-cwHWhAbXrl2q62iLVTjiyEZ7O7n-CROOY494k7Q3xrs_w",
+            "response": {
+                "attestationObject": "",
+                "clientDataJSON": "eyJ0eXBlIjoid2ViYXV0aG4uY3JlYXRlIiwiY2hhbGxlbmdlIjoiVHdON240V1R5R0tMYzRaWS1xR3NGcUtuSE00bmdscXN5VjBJQ0psTjJUTzlYaVJ5RnRya2FEd1V2c3FsLWdrTEpYUDZmbkYxTWxyWjUzTW00UjdDdnciLCJvcmlnaW4iOiJodHRwOi8vbG9jYWxob3N0OjUwMDAiLCJjcm9zc09yaWdpbiI6ZmFsc2V9"
+            },
+            "type": "public-key",
+            "clientExtensionResults": {},
+            "transports": [
+                "cable"
+            ]
+        }
+
+        challenge = base64url_to_bytes(
+            "TwN7n4WTyGKLc4ZY-qGsFqKnHM4nglqsyV0ICJlN2TO9XiRyFtrkaDwUvsql-gkLJXP6fnF1MlrZ53Mm4R7Cvw"
+        )
+        rp_id = "localhost"
+        expected_origin = "http://localhost:5000"
+
+        with self.assertRaises(InvalidCBORData):
+            verify_registration_response(
+                credential=credential,
+                expected_challenge=challenge,
+                expected_origin=expected_origin,
+                expected_rp_id=rp_id,
+            )
diff --git a/webauthn/helpers/parse_attestation_object.py b/webauthn/helpers/parse_attestation_object.py
index 042899a..42ce801 100644
--- a/webauthn/helpers/parse_attestation_object.py
+++ b/webauthn/helpers/parse_attestation_object.py
@@ -1,8 +1,7 @@
-import cbor2
-
 from .parse_attestation_statement import parse_attestation_statement
 from .parse_authenticator_data import parse_authenticator_data
 from .structs import AttestationObject
+from .parse_cbor import parse_cbor
 
 
 def parse_attestation_object(val: bytes) -> AttestationObject:
@@ -10,7 +9,7 @@ def parse_attestation_object(val: bytes) -> AttestationObject:
     Decode and peel apart the CBOR-encoded blob `response.attestationObject` into
     structured data.
     """
-    attestation_dict = cbor2.loads(val)
+    attestation_dict = parse_cbor(val)
 
     decoded_attestation_object = AttestationObject(
         fmt=attestation_dict["fmt"],