From: tb <tb@openbsd.org>
Date: Tue, 4 Jul 2023 15:09:31 +0000 (+0000)
Subject: Avoid outputting invalid signatures
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=5dfb914b63d9dac6504bd370f3d9accd4cb0421f;p=openbsd

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
---

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;