diff options
author | Petr Gotthard <petr.gotthard@centrum.cz> | 2021-02-06 21:47:20 +0100 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2021-02-09 11:15:55 +0100 |
commit | 604b86d8d360e36fc2fc0d1611d05bf38699d297 (patch) | |
tree | 5969f02f05293556906af2a19a4a3066b331f8ad /crypto/params_from_text.c | |
parent | e60a748a13a244e8b13bacca18bad9bb3505aa90 (diff) | |
download | openssl-new-604b86d8d360e36fc2fc0d1611d05bf38699d297.tar.gz |
Enhanced integer parsing in OSSL_PARAM_allocate_from_text
Fixes #14041 and additional bugs discovered by the newly created
tests.
This patch:
- Introduces support for 0x prefixed integers
- Fixes parsing of negative integers (negative numbers were
shifted by -2)
- Fixes ability to parse maximal unsigned numbers ("too small
buffer" error used to be reported incorrectly)
- Fixes a memory leak when OSSL_PARAM_allocate_from_text fails
leaving a temporary BN allocated
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14093)
Diffstat (limited to 'crypto/params_from_text.c')
-rw-r--r-- | crypto/params_from_text.c | 21 |
1 files changed, 15 insertions, 6 deletions
diff --git a/crypto/params_from_text.c b/crypto/params_from_text.c index ddc3c38aa4..b019744f9b 100644 --- a/crypto/params_from_text.c +++ b/crypto/params_from_text.c @@ -28,6 +28,7 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key, size_t *buf_n, BIGNUM **tmpbn, int *found) { const OSSL_PARAM *p; + size_t buf_bits; /* * ishex is used to translate legacy style string controls in hex format @@ -50,7 +51,7 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key, if (*ishex) BN_hex2bn(tmpbn, value); else - BN_dec2bn(tmpbn, value); + BN_asc2bn(tmpbn, value); if (*tmpbn == NULL) return 0; @@ -62,20 +63,25 @@ static int prepare_from_text(const OSSL_PARAM *paramdefs, const char *key, * buffer, i.e. if it's negative, we need to deal with it. We do * it by subtracting 1 here and inverting the bytes in * construct_from_text() below. + * To subtract 1 from an absolute value of a negative number we + * actually have to add 1: -3 - 1 = -4, |-3| = 3 + 1 = 4. */ if (p->data_type == OSSL_PARAM_INTEGER && BN_is_negative(*tmpbn) - && !BN_sub_word(*tmpbn, 1)) { + && !BN_add_word(*tmpbn, 1)) { return 0; } - *buf_n = BN_num_bytes(*tmpbn); + buf_bits = (size_t)BN_num_bits(*tmpbn); + *buf_n = (buf_bits + 7) / 8; /* * TODO(v3.0) is this the right way to do this? This code expects * a zero data size to simply mean "arbitrary size". */ if (p->data_size > 0) { - if (*buf_n >= p->data_size) { + if (buf_bits > p->data_size * 8 + || (p->data_type == OSSL_PARAM_INTEGER + && buf_bits == p->data_size * 8)) { ERR_raise(ERR_LIB_CRYPTO, CRYPTO_R_TOO_SMALL_BUFFER); /* Since this is a different error, we don't break */ return 0; @@ -184,11 +190,11 @@ int OSSL_PARAM_allocate_from_text(OSSL_PARAM *to, if (!prepare_from_text(paramdefs, key, value, value_n, ¶mdef, &ishex, &buf_n, &tmpbn, found)) - return 0; + goto err; if ((buf = OPENSSL_zalloc(buf_n > 0 ? buf_n : 1)) == NULL) { ERR_raise(ERR_LIB_CRYPTO, ERR_R_MALLOC_FAILURE); - return 0; + goto err; } ok = construct_from_text(to, paramdef, value, value_n, ishex, @@ -197,4 +203,7 @@ int OSSL_PARAM_allocate_from_text(OSSL_PARAM *to, if (!ok) OPENSSL_free(buf); return ok; + err: + BN_free(tmpbn); + return 0; } |