From: schwarze <schwarze@openbsd.org>
Date: Sun, 20 Nov 2022 23:35:00 +0000 (+0000)
Subject: Fix a surprising quirk in BN_GF2m_mod(3).
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=bf9201ee11afd5f5c4794b9e7c234feb941e52c0;p=openbsd

Fix a surprising quirk in BN_GF2m_mod(3).

All other wrappers in the same file that use a temporary array of
degrees size that array dynamically, such that they are able to
handle reducing polynomials of arbitrary lengths.  BN_GF2m_mod(3)
was the only one that used a static array of size 6 instead, limiting
it to trinomials and pentanomials and causing it to fail for longer
reducing polynomials.

Make this more uniform and less surprising by using exactly the
same code as in all the other wrappers, such that BN_GF2m_mod(3)
works with reducing polynomials of arbitrary length, too, just like
the others.

Again, tb@ points out this quirk is very unlikely to cause
vulnerabilities in practice because cryptographic applications do
not use longer reducing polynomials.

This patch is not expected to significantly impact performance
because the relevant caller, BN_GF2m_mod_div(3), already uses dynamic
allocation via BN_GF2m_mod_mul(3).

OK tb@
---

diff --git a/lib/libcrypto/bn/bn_gf2m.c b/lib/libcrypto/bn/bn_gf2m.c
index 1fd7105a313..b9e3ba8566b 100644
--- a/lib/libcrypto/bn/bn_gf2m.c
+++ b/lib/libcrypto/bn/bn_gf2m.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: bn_gf2m.c,v 1.24 2022/11/20 22:23:43 schwarze Exp $ */
+/* $OpenBSD: bn_gf2m.c,v 1.25 2022/11/20 23:35:00 schwarze Exp $ */
 /* ====================================================================
  * Copyright 2002 Sun Microsystems, Inc. ALL RIGHTS RESERVED.
  *
@@ -464,17 +464,23 @@ int
 BN_GF2m_mod(BIGNUM *r, const BIGNUM *a, const BIGNUM *p)
 {
 	int ret = 0;
-	int arr[6];
+	const int max = BN_num_bits(p) + 1;
+	int *arr = NULL;
 
 	bn_check_top(a);
 	bn_check_top(p);
-	ret = BN_GF2m_poly2arr(p, arr, sizeof(arr) / sizeof(arr[0]));
-	if (!ret || ret > (int)(sizeof(arr) / sizeof(arr[0]))) {
+	if ((arr = reallocarray(NULL, max, sizeof(int))) == NULL)
+		goto err;
+	ret = BN_GF2m_poly2arr(p, arr, max);
+	if (!ret || ret > max) {
 		BNerror(BN_R_INVALID_LENGTH);
-		return 0;
+		goto err;
 	}
 	ret = BN_GF2m_mod_arr(r, a, arr);
 	bn_check_top(r);
+
+ err:
+	free(arr);
 	return ret;
 }
 
diff --git a/lib/libcrypto/man/BN_GF2m_add.3 b/lib/libcrypto/man/BN_GF2m_add.3
index 170a7df18ac..60a2a5eed4a 100644
--- a/lib/libcrypto/man/BN_GF2m_add.3
+++ b/lib/libcrypto/man/BN_GF2m_add.3
@@ -1,4 +1,4 @@
-.\" $OpenBSD: BN_GF2m_add.3,v 1.3 2022/11/18 07:28:34 tb Exp $
+.\" $OpenBSD: BN_GF2m_add.3,v 1.4 2022/11/20 23:35:00 schwarze Exp $
 .\"
 .\" Copyright (c) 2022 Ingo Schwarze <schwarze@openbsd.org>
 .\"
@@ -14,7 +14,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: November 18 2022 $
+.Dd $Mdocdate: November 20 2022 $
 .Dt BN_GF2M_ADD 3
 .Os
 .Sh NAME
@@ -480,9 +480,7 @@ In one of the functions wrapping an
 .Fn *_arr
 variant, the
 .Fa "BIGNUM *p"
-argument had a value of zero, or in
-.Fn BN_GF2m_mod ,
-it contained more than five non-zero coefficients.
+argument had a value of zero.
 .El
 .Sh SEE ALSO
 .Xr BN_add 3 ,
@@ -514,9 +512,3 @@ it contained more than five non-zero coefficients.
  exponentiation algorithm A.4.1 for square roots, and\
  algorithms A.4.7 and A.4.6 for the quadratic equation
 .Re
-.Sh BUGS
-.Fn BN_GF2m_mod
-is arbitrarily limited to reducing polynomials containing at most five
-non-zero coefficients and returns failure if
-.Fa p
-contains six or more non-zero coefficients.