From 2b49ca11f06dee99e79f9f92acb8cd0e45cc5878 Mon Sep 17 00:00:00 2001 From: Bence Mali Date: Mon, 13 May 2024 15:35:24 +0200 Subject: [PATCH] DECODE_UINT32 without lengths checked fixed Signed-off-by: Bence Mali Signed-off-by: Michael Baentsch <57787676+baentsch@users.noreply.github.com> --- oqsprov/oqs_encode_key2any.c | 40 +++++++++++++++++---- oqsprov/oqs_kmgmt.c | 4 +-- oqsprov/oqs_sig.c | 22 ++++++++++-- oqsprov/oqsprov_keys.c | 69 +++++++++++++++++++++++++++++++----- 4 files changed, 115 insertions(+), 20 deletions(-) diff --git a/oqsprov/oqs_encode_key2any.c b/oqsprov/oqs_encode_key2any.c index b06e6138..2c0e206e 100644 --- a/oqsprov/oqs_encode_key2any.c +++ b/oqsprov/oqs_encode_key2any.c @@ -617,7 +617,7 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) { OQSX_KEY *oqsxkey = (OQSX_KEY *)vxkey; unsigned char *buf = NULL; - int buflen = 0, privkeylen; + uint32_t buflen = 0, privkeylen = 0; ASN1_OCTET_STRING oct; int keybloblen, nid; STACK_OF(ASN1_TYPE) *sk = NULL; @@ -643,9 +643,14 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) if (oqsxkey->keytype != KEY_TYPE_CMP_SIG) { privkeylen = oqsxkey->privkeylen; if (oqsxkey->numkeys > 1) { // hybrid - int actualprivkeylen; + uint32_t actualprivkeylen = 0; + size_t fixed_pq_privkeylen + = oqsxkey->oqsx_provider_ctx.oqsx_qs_ctx.kem->length_secret_key; + size_t space_for_classical_privkey + = privkeylen - SIZE_OF_UINT32 - fixed_pq_privkeylen; DECODE_UINT32(actualprivkeylen, oqsxkey->privkey); - if (actualprivkeylen > oqsxkey->evp_info->length_private_key) { + if ((actualprivkeylen > oqsxkey->evp_info->length_private_key) + || (actualprivkeylen > space_for_classical_privkey)) { ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); return 0; } @@ -690,7 +695,7 @@ static int oqsx_pki_priv_to_der(const void *vxkey, unsigned char **pder) ERR_raise(ERR_LIB_USER, ERR_R_MALLOC_FAILURE); return -1; } - OQS_ENC_PRINTF2("OQS ENC provider: saving privkey of length %d\n", + OQS_ENC_PRINTF2("OQS ENC provider: saving privkey of length %zu\n", buflen); memcpy(buf, oqsxkey->privkey, privkeylen); #else @@ -1726,7 +1731,8 @@ static int oqsx_to_text(BIO *out, const void *key, int selection) if (okey->keytype == KEY_TYPE_CMP_SIG) { char *name; char label[200]; - int i, privlen; + int i; + uint32_t privlen = 0; for (i = 0; i < okey->numkeys; i++) { if ((name = get_cmpname(OBJ_sn2nid(okey->tls_name), i)) == NULL) { @@ -1761,10 +1767,20 @@ static int oqsx_to_text(BIO *out, const void *key, int selection) } else { if (okey->numkeys > 1) { // hybrid key char classic_label[200]; - int classic_key_len = 0; + uint32_t classic_key_len = 0; + size_t fixed_pq_privkey_len + = okey->oqsx_provider_ctx.oqsx_qs_ctx.kem + ->length_secret_key; + size_t space_for_classical_privkey = okey->privkeylen + - SIZE_OF_UINT32 + - fixed_pq_privkey_len; sprintf(classic_label, "%s key material:", OBJ_nid2sn(okey->evp_info->nid)); DECODE_UINT32(classic_key_len, okey->privkey); + if (classic_key_len > space_for_classical_privkey) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + return 0; + } if (!print_labeled_buf(out, classic_label, okey->comp_privkey[0], classic_key_len)) @@ -1809,8 +1825,18 @@ static int oqsx_to_text(BIO *out, const void *key, int selection) } else { if (okey->numkeys > 1) { // hybrid key char classic_label[200]; - int classic_key_len = 0; + uint32_t classic_key_len = 0; + size_t fixed_pq_pubkey_len + = okey->oqsx_provider_ctx.oqsx_qs_ctx.kem + ->length_public_key; + size_t space_for_classical_pubkey = okey->pubkeylen + - SIZE_OF_UINT32 + - fixed_pq_pubkey_len; DECODE_UINT32(classic_key_len, okey->pubkey); + if (classic_key_len > space_for_classical_pubkey) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + return 0; + } sprintf(classic_label, "%s key material:", OBJ_nid2sn(okey->evp_info->nid)); if (!print_labeled_buf(out, classic_label, diff --git a/oqsprov/oqs_kmgmt.c b/oqsprov/oqs_kmgmt.c index 50ad3012..01c7de5d 100644 --- a/oqsprov/oqs_kmgmt.c +++ b/oqsprov/oqs_kmgmt.c @@ -364,8 +364,8 @@ static int oqsx_get_hybrid_params(OQSX_KEY *key, OSSL_PARAM params[]) const void *classical_privkey = NULL; const void *pq_pubkey = NULL; const void *pq_privkey = NULL; - int classical_pubkey_len = 0; - int classical_privkey_len = 0; + uint32_t classical_pubkey_len = 0; + uint32_t classical_privkey_len = 0; int pq_pubkey_len = 0; int pq_privkey_len = 0; diff --git a/oqsprov/oqs_sig.c b/oqsprov/oqs_sig.c index c3d43391..31ec99cc 100644 --- a/oqsprov/oqs_sig.c +++ b/oqsprov/oqs_sig.c @@ -651,9 +651,13 @@ static int oqs_sig_verify(void *vpoqs_sigctx, const unsigned char *sig, if (is_hybrid) { const EVP_MD *classical_md; - size_t actual_classical_sig_len = 0; + uint32_t actual_classical_sig_len = 0; int digest_len; unsigned char digest[SHA512_DIGEST_LENGTH]; /* init with max length */ + size_t max_pq_sig_len + = oqsxkey->oqsx_provider_ctx.oqsx_qs_ctx.sig->length_signature; + size_t max_classical_sig_len = oqsxkey->oqsx_provider_ctx.oqsx_evp_ctx + ->evp_info->length_signature; if ((ctx_verify = EVP_PKEY_CTX_new(oqsxkey->classical_pkey, NULL)) == NULL @@ -668,7 +672,21 @@ static int oqs_sig_verify(void *vpoqs_sigctx, const unsigned char *sig, goto endverify; } } - DECODE_UINT32(actual_classical_sig_len, sig); + if (siglen > SIZE_OF_UINT32) { + size_t actual_pq_sig_len = 0; + DECODE_UINT32(actual_classical_sig_len, sig); + actual_pq_sig_len + = siglen - SIZE_OF_UINT32 - actual_classical_sig_len; + if (siglen <= (SIZE_OF_UINT32 + actual_classical_sig_len) + || actual_classical_sig_len > max_classical_sig_len + || actual_pq_sig_len > max_pq_sig_len) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto endverify; + } + } else { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto endverify; + } /* same as with sign: activate if pre-existing hashing to be used: * if (poqs_sigctx->mdctx == NULL) { // hashing not yet done diff --git a/oqsprov/oqsprov_keys.c b/oqsprov/oqsprov_keys.c index c20b3892..870ccc38 100644 --- a/oqsprov/oqsprov_keys.c +++ b/oqsprov/oqsprov_keys.c @@ -317,11 +317,16 @@ static int oqsx_key_set_composites(OQSX_KEY *key) } } } else { - int classic_pubkey_len, classic_privkey_len; + uint32_t classic_pubkey_len = 0; + uint32_t classic_privkey_len = 0; if (key->privkey) { key->comp_privkey[0] = (char *)key->privkey + SIZE_OF_UINT32; DECODE_UINT32(classic_privkey_len, key->privkey); + if (classic_privkey_len > key->evp_info->length_private_key) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err; + } key->comp_privkey[1] = (char *)key->privkey + classic_privkey_len + SIZE_OF_UINT32; } else { @@ -331,6 +336,10 @@ static int oqsx_key_set_composites(OQSX_KEY *key) if (key->pubkey) { key->comp_pubkey[0] = (char *)key->pubkey + SIZE_OF_UINT32; DECODE_UINT32(classic_pubkey_len, key->pubkey); + if (classic_pubkey_len > key->evp_info->length_public_key) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err; + } key->comp_pubkey[1] = (char *)key->pubkey + classic_pubkey_len + SIZE_OF_UINT32; } else { @@ -649,13 +658,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p, } #endif } else { - int classical_privatekey_len = 0; + uint32_t classical_privatekey_len = 0; // for plain OQS keys, we expect OQS priv||OQS pub key size_t actualprivkeylen = key->privkeylen; // for hybrid keys, we expect classic priv key||OQS priv key||OQS pub // key classic pub key must/can be re-created from classic private key if (key->keytype == KEY_TYPE_CMP_SIG) { - size_t privlen = 0; + uint32_t privlen = 0; size_t publen = 0; size_t previous_privlen = 0; size_t previous_publen = 0; @@ -720,6 +729,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p, // keys. will recreate the pubkey later if (key->oqsx_provider_ctx.oqsx_evp_ctx->evp_info->keytype == EVP_PKEY_RSA) { // get the RSA real key size + if (previous_privlen + previous_publen + 4 > plen) { + OPENSSL_free(name); + OPENSSL_secure_clear_free(temp_priv, temp_priv_len); + OPENSSL_secure_clear_free(temp_pub, temp_pub_len); + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err_key_op; + } unsigned char *enc_len = OPENSSL_strndup( p + previous_privlen + previous_publen, 4); OPENSSL_cleanse(enc_len, 2); @@ -743,6 +759,13 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p, else publen = 0; } + if (previous_privlen + previous_publen + privlen > plen) { + OPENSSL_free(name); + OPENSSL_secure_clear_free(temp_priv, temp_priv_len); + OPENSSL_secure_clear_free(temp_pub, temp_pub_len); + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err_key_op; + } memcpy(temp_priv + previous_privlen, p + previous_privlen + previous_publen, privlen); memcpy(temp_pub + previous_publen, @@ -758,11 +781,30 @@ static OQSX_KEY *oqsx_key_op(const X509_ALGOR *palg, const unsigned char *p, OPENSSL_secure_clear_free(temp_pub, temp_pub_len); } else { if (key->numkeys == 2) { - DECODE_UINT32(classical_privatekey_len, - p); // actual classic key len - // adjust expected size - if (classical_privatekey_len - > key->evp_info->length_private_key) { + size_t expected_pq_privkey_len + = key->oqsx_provider_ctx.oqsx_qs_ctx.kem->length_secret_key; +#ifndef NOPUBKEY_IN_PRIVKEY + expected_pq_privkey_len += key->oqsx_provider_ctx.oqsx_qs_ctx + .kem->length_public_key; +#endif + if (plen > (SIZE_OF_UINT32 + expected_pq_privkey_len)) { + size_t max_classical_privkey_len + = key->evp_info->length_private_key; + size_t space_for_classical_privkey + = plen - expected_pq_privkey_len - SIZE_OF_UINT32; + if (space_for_classical_privkey + > max_classical_privkey_len) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err_key_op; + } + DECODE_UINT32(classical_privatekey_len, + p); // actual classic key len + if (classical_privatekey_len + != space_for_classical_privkey) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto err_key_op; + } + } else { ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); goto err_key_op; } @@ -970,7 +1012,8 @@ static int oqsx_key_recreate_classickey(OQSX_KEY *key, oqsx_key_op_t op) } } else { if (key->numkeys == 2) { // hybrid key - int classical_pubkey_len, classical_privkey_len; + uint32_t classical_pubkey_len = 0; + uint32_t classical_privkey_len = 0; if (!key->evp_info) { ERR_raise(ERR_LIB_USER, OQSPROV_R_EVPINFO_MISSING); goto rec_err; @@ -978,6 +1021,10 @@ static int oqsx_key_recreate_classickey(OQSX_KEY *key, oqsx_key_op_t op) if (op == KEY_OP_PUBLIC) { const unsigned char *enc_pubkey = key->comp_pubkey[0]; DECODE_UINT32(classical_pubkey_len, key->pubkey); + if (classical_pubkey_len > key->evp_info->length_public_key) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto rec_err; + } if (key->evp_info->raw_key_support) { key->classical_pkey = EVP_PKEY_new_raw_public_key( key->evp_info->keytype, NULL, enc_pubkey, @@ -1003,6 +1050,10 @@ static int oqsx_key_recreate_classickey(OQSX_KEY *key, oqsx_key_op_t op) } if (op == KEY_OP_PRIVATE) { DECODE_UINT32(classical_privkey_len, key->privkey); + if (classical_privkey_len > key->evp_info->length_private_key) { + ERR_raise(ERR_LIB_USER, OQSPROV_R_INVALID_ENCODING); + goto rec_err; + } const unsigned char *enc_privkey = key->comp_privkey[0]; unsigned char *enc_pubkey = key->comp_pubkey[0]; if (key->evp_info->raw_key_support) {