From c7aa2b87a4e2f8a8326e169a7ba35833545be8ee Mon Sep 17 00:00:00 2001 From: Pol Henarejos Date: Fri, 14 Aug 2026 12:59:55 +0200 Subject: [PATCH] Improve omitted field checks. Signed-off-by: Pol Henarejos --- src/fido/cbor_client_pin.c | 7 ++++--- src/fido/cbor_config.c | 10 ++++++---- src/fido/cbor_cred_mgmt.c | 10 +++++++--- src/fido/cbor_get_assertion.c | 11 ++++++----- src/fido/cbor_large_blobs.c | 9 +++++---- src/fido/cbor_make_credential.c | 31 ++++++++++++++++--------------- 6 files changed, 44 insertions(+), 34 deletions(-) diff --git a/src/fido/cbor_client_pin.c b/src/fido/cbor_client_pin.c index a5ba13b..f5a025b 100644 --- a/src/fido/cbor_client_pin.c +++ b/src/fido/cbor_client_pin.c @@ -432,6 +432,10 @@ int cbor_client_pin(const uint8_t *data, size_t len) { } CBOR_PARSE_MAP_END(map, 1); + if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + cbor_encoder_init(&encoder, ctap_resp->init.data + 1, CTAP_MAX_CBOR_PAYLOAD, 0); if (subcommand == 0x0) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); @@ -454,9 +458,6 @@ int cbor_client_pin(const uint8_t *data, size_t len) { CBOR_CHECK(COSE_key_shared(&hkey, &mapEncoder, &mapEncoder2)); } - else if (pinUvAuthProtocol == 0) { - CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); - } else { CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); } diff --git a/src/fido/cbor_config.c b/src/fido/cbor_config.c index 3a7b830..22501f5 100644 --- a/src/fido/cbor_config.c +++ b/src/fido/cbor_config.c @@ -43,6 +43,7 @@ int cbor_config(const uint8_t *data, size_t len) { //CborEncoder mapEncoder; uint8_t *raw_subpara = NULL; const bool *forceChangePin = NULL, *pinPolicy = NULL; + bool pinUvAuthProtocol_present = false; CBOR_CHECK(cbor_parser_init(data, len, 0, &parser, &map)); uint64_t val_c = 1; @@ -109,6 +110,7 @@ int cbor_config(const uint8_t *data, size_t len) { } else if (val_u == 0x03) { CBOR_FIELD_GET_UINT(pinUvAuthProtocol, 1); + pinUvAuthProtocol_present = true; } else if (val_u == 0x04) { CBOR_FIELD_GET_BYTES(pinUvAuthParam, 1); @@ -125,15 +127,15 @@ int cbor_config(const uint8_t *data, size_t len) { cbor_encoder_init(&encoder, ctap_resp->init.data + 1, CTAP_MAX_CBOR_PAYLOAD, 0); + if (pinUvAuthProtocol_present && pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } if (pinUvAuthParam.present == false) { CBOR_ERROR(CTAP2_ERR_PUAT_REQUIRED); } - if (pinUvAuthProtocol == 0) { + if (pinUvAuthProtocol_present == false) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } - if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } size_t expected_auth_len = pinUvAuthProtocol == 1 ? 16u : 32u; if (pinUvAuthParam.len != expected_auth_len) { CBOR_ERROR(CTAP2_ERR_PIN_AUTH_INVALID); diff --git a/src/fido/cbor_cred_mgmt.c b/src/fido/cbor_cred_mgmt.c index 595245e..e77721a 100644 --- a/src/fido/cbor_cred_mgmt.c +++ b/src/fido/cbor_cred_mgmt.c @@ -68,7 +68,7 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) { CborEncoder encoder, mapEncoder, mapEncoder2; uint8_t *raw_subpara = NULL; size_t raw_subpara_len = 0; - bool asserted = false, is_preview = *(data - 1) == 0x41; // Backwards compatibility + bool asserted = false, is_preview = *(data - 1) == 0x41, pinUvAuthProtocol_present = false; // Backwards compatibility CBOR_CHECK(cbor_parser_init(data, len, 0, &parser, &map)); uint64_t val_c = 1; @@ -132,6 +132,7 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) { } else if (val_u == 0x03) { CBOR_FIELD_GET_UINT(pinUvAuthProtocol, 1); + pinUvAuthProtocol_present = true; } else if (val_u == 0x04) { // pubKeyCredParams CBOR_FIELD_GET_BYTES(pinUvAuthParam, 1); @@ -143,12 +144,15 @@ int cbor_cred_mgmt(const uint8_t *data, size_t len) { CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); } + if (pinUvAuthProtocol_present && pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } if (subcommand != 0x03 && subcommand != 0x05) { if (pinUvAuthParam.present == false) { CBOR_ERROR(CTAP2_ERR_PUAT_REQUIRED); } - if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + if (pinUvAuthProtocol_present == false) { + CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } } diff --git a/src/fido/cbor_get_assertion.c b/src/fido/cbor_get_assertion.c index f7f2093..612f910 100644 --- a/src/fido/cbor_get_assertion.c +++ b/src/fido/cbor_get_assertion.c @@ -104,7 +104,7 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) { Credential creds[MAX_CREDENTIAL_COUNT_IN_LIST] = { 0 }; size_t allowList_len = 0, creds_len = 0; uint8_t *aut_data = NULL; - bool asserted = false, up = false, uv = false; + bool asserted = false, up = false, uv = false, pinUvAuthProtocol_present = false; int64_t kty = 2, alg = 0, crv = 0; CborByteString kax = { 0 }, kay = { 0 }, salt_enc = { 0 }, salt_auth = { 0 }; const bool *credBlob = NULL; @@ -211,6 +211,7 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) { } else if (val_u == 0x07) { // pinUvAuthProtocol CBOR_FIELD_GET_UINT(pinUvAuthProtocol, 1); + pinUvAuthProtocol_present = true; } } CBOR_PARSE_MAP_END(map, 1); @@ -218,6 +219,9 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) { if (rpId.present == false || clientDataHash.present == false) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } + if (pinUvAuthProtocol_present && pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } rp_id = rpId.data; user_name = NULL; display_name = NULL; @@ -249,12 +253,9 @@ int cbor_get_assertion(const uint8_t *data, size_t len, bool next) { } } else { - if (pinUvAuthProtocol == 0) { + if (pinUvAuthProtocol_present == false) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } - if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } } } if (options.present) { diff --git a/src/fido/cbor_large_blobs.c b/src/fido/cbor_large_blobs.c index 5d9cd89..e5ce2e5 100644 --- a/src/fido/cbor_large_blobs.c +++ b/src/fido/cbor_large_blobs.c @@ -86,6 +86,10 @@ int cbor_large_blobs(const uint8_t *data, size_t len) { } CBOR_PARSE_MAP_END(map, 1); + if (pinUvAuthProtocol_present && pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (offset == UINT64_MAX) { CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); } @@ -144,12 +148,9 @@ int cbor_large_blobs(const uint8_t *data, size_t len) { if (pinUvAuthParam.present == false) { CBOR_ERROR(CTAP2_ERR_PUAT_REQUIRED); } - if (pinUvAuthProtocol == 0) { + if (pinUvAuthProtocol_present == false) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } - if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } size_t expected_auth_len = pinUvAuthProtocol == 1 ? 16u : 32u; if (pinUvAuthParam.len != expected_auth_len) { CBOR_ERROR(CTAP2_ERR_PIN_AUTH_INVALID); diff --git a/src/fido/cbor_make_credential.c b/src/fido/cbor_make_credential.c index 580580e..c4c6256 100644 --- a/src/fido/cbor_make_credential.c +++ b/src/fido/cbor_make_credential.c @@ -66,7 +66,7 @@ int cbor_make_credential(const uint8_t *data, size_t len) { uint64_t pinUvAuthProtocol = 0, enterpriseAttestation = 0, hmacSecretPinUvAuthProtocol = 1; int64_t kty = 2, hmac_alg = 0, crv = 0; CborByteString kax = { 0 }, kay = { 0 }, salt_enc = { 0 }, salt_auth = { 0 }; - bool hmac_secret_mc = false, has_credprot = false; + bool hmac_secret_mc = false, has_credprot = false, pinUvAuthProtocol_present = false, enterpriseAttestation_present = false; const bool *pin_complexity_policy = NULL, *uvm = NULL; uint8_t *aut_data = NULL; size_t resp_size = 0; @@ -223,12 +223,26 @@ int cbor_make_credential(const uint8_t *data, size_t len) { } else if (val_u == 0x09) { // pinUvAuthProtocol CBOR_FIELD_GET_UINT(pinUvAuthProtocol, 1); + pinUvAuthProtocol_present = true; } else if (val_u == 0x0A) { // enterpriseAttestation CBOR_FIELD_GET_UINT(enterpriseAttestation, 1); + enterpriseAttestation_present = true; } } CBOR_PARSE_MAP_END(map, 1); + if (pinUvAuthProtocol_present && pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (enterpriseAttestation_present) { + file_t *ef_ee_ea = file_search_by_fid(EF_EE_DEV_EA, NULL, SPECIFY_EF); + if (!(get_opts() & FIDO2_OPT_EA) || !file_has_data(ef_ee_ea)) { + CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); + } + if (enterpriseAttestation != 1 && enterpriseAttestation != 2) { + CBOR_ERROR(CTAP2_ERR_INVALID_OPTION); + } + } if (hmac_secret_mc && extensions.hmac_secret != ptrue) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } @@ -272,12 +286,9 @@ int cbor_make_credential(const uint8_t *data, size_t len) { } } else { - if (pinUvAuthProtocol == 0) { + if (pinUvAuthProtocol_present == false) { CBOR_ERROR(CTAP2_ERR_MISSING_PARAMETER); } - if (pinUvAuthProtocol != 1 && pinUvAuthProtocol != 2) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } } } @@ -397,16 +408,6 @@ int cbor_make_credential(const uint8_t *data, size_t len) { if (has_credprot == true && (extensions.credProtect < CRED_PROT_UV_OPTIONAL || extensions.credProtect > CRED_PROT_UV_REQUIRED)) { CBOR_ERROR(CTAP2_ERR_INVALID_OPTION); } - if (enterpriseAttestation > 0) { - file_t *ef_ee_ea = file_search_by_fid(EF_EE_DEV_EA, NULL, SPECIFY_EF); - if (!(get_opts() & FIDO2_OPT_EA) || !file_has_data(ef_ee_ea)) { - CBOR_ERROR(CTAP1_ERR_INVALID_PARAMETER); - } - if (enterpriseAttestation != 1 && enterpriseAttestation != 2) { //9.2.1 - CBOR_ERROR(CTAP2_ERR_INVALID_OPTION); - } - //Unfinished. See 6.1.2.9 - } if (!((get_opts() & FIDO2_OPT_MCUV_NOTRQD) && options.rk != ptrue && options.uv != ptrue && pinUvAuthParam.present == false)) { //10.1 if (pinUvAuthParam.present == true) { //11.1 int ret = verify((uint8_t)pinUvAuthProtocol, paut.data, clientDataHash.data, (uint16_t)clientDataHash.len, pinUvAuthParam.data);