From 5dfb914b63d9dac6504bd370f3d9accd4cb0421f Mon Sep 17 00:00:00 2001 From: tb Date: Tue, 4 Jul 2023 15:09:31 +0000 Subject: [PATCH] Avoid outputting invalid signatures The caller can provide an r which will be added to the ECDSA_SIG unchecked. This can happen via ECDSA_{,do_}sign_ex() or ECDSA_sign_setup() or else via a custom sign_sig() handler. Therefore add a check that it is in the bounds required. Since k was long thrown away, there's no way to check kinv, so it needs to be trusted. Misdesigned APIs that will output garbage everywhere... ok jsing --- lib/libcrypto/ecdsa/ecs_ossl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/libcrypto/ecdsa/ecs_ossl.c b/lib/libcrypto/ecdsa/ecs_ossl.c index de51d3aa4a8..0ca2651f255 100644 --- a/lib/libcrypto/ecdsa/ecs_ossl.c +++ b/lib/libcrypto/ecdsa/ecs_ossl.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ecs_ossl.c,v 1.70 2023/07/04 14:59:32 tb Exp $ */ +/* $OpenBSD: ecs_ossl.c,v 1.71 2023/07/04 15:09:31 tb Exp $ */ /* * Written by Nils Larsch for the OpenSSL project */ @@ -316,6 +316,16 @@ ecdsa_compute_s(BIGNUM **out_s, const BIGNUM *e, const BIGNUM *kinv, if ((s = BN_new()) == NULL) goto err; + /* + * In a valid ECDSA signature, r must be in [1, order). Since r can be + * caller provided - either directly or by replacing sign_setup() - we + * can't rely on this being the case. + */ + if (BN_cmp(r, BN_value_one()) < 0 || BN_cmp(r, order) >= 0) { + ECDSAerror(ECDSA_R_BAD_SIGNATURE); + goto err; + } + if (!bn_rand_interval(b, BN_value_one(), order)) { ECDSAerror(ERR_R_BN_LIB); goto err; -- 2.20.1