From: "Theo de Raadt" Subject: Re: dc, bc and BN_get_word() To: Theo Buehler Cc: tech@openbsd.org, otto@openbsd.org Date: Sat, 06 Dec 2025 08:33:35 -0700 That looks a lot better, especially that it is done inside the library. - if ((cofactor_word = BN_get_word(cofactor)) == BN_MASK2) + if ((cofactor_word = BN_get_word(cofactor)) == (BN_ULONG)-1) Theo Buehler wrote: > I would normally have dealt with this before committing to bn.h, but I > forgot about moving this out of the way before commit... > > I removed BN_MASK2 from bn.h, where it was part of a more complicated > version of the following after relatively recent changes by jsing: > > #if defined(_LP64) > #define BN_ULONG uint64_t > #define BN_MASK2 INT64_C(0xffffffffffffffff) > #else > #define BN_ULONG uint32_t > #define BN_MASK2 INT32_C(0xffffffff) > #endif > > Now BN_MASK2 is used internally in libcrypto as a mask (like the > name indicates), except in one place where it is used for the error > check of BN_get_word(). The only other consumers in base and ports are > bc and dc. > > The manual is a bit vague on how to check for this error. I think it > a comparison against an explicit value. > > So the diff below removes the workaround I added earlier today, makes > the manual more explicit and changes the checks in libcrypto and dc > accordingly. > > Index: lib/libcrypto/man/BN_zero.3 > =================================================================== > RCS file: /cvs/src/lib/libcrypto/man/BN_zero.3,v > diff -u -p -r1.15 BN_zero.3 > --- lib/libcrypto/man/BN_zero.3 14 Jun 2025 06:48:47 -0000 1.15 > +++ lib/libcrypto/man/BN_zero.3 6 Dec 2025 09:38:13 -0000 > @@ -132,7 +132,7 @@ This constant is useful for comparisons > .Fn BN_get_word > returns the value > .Fa a , > -or a number with all bits set if > +or (BN_ULONG)\-1 if > .Fa a > cannot be represented as a > .Vt BN_ULONG . > Index: lib/libcrypto/ec/ec_curve.c > =================================================================== > RCS file: /cvs/src/lib/libcrypto/ec/ec_curve.c,v > diff -u -p -r1.59 ec_curve.c > --- lib/libcrypto/ec/ec_curve.c 2 Nov 2025 13:06:30 -0000 1.59 > +++ lib/libcrypto/ec/ec_curve.c 6 Dec 2025 09:38:27 -0000 > @@ -1392,7 +1392,7 @@ ec_curve_from_group(const EC_GROUP *grou > if ((cofactor = EC_GROUP_get0_cofactor(group)) != NULL) { > BN_ULONG cofactor_word; > > - if ((cofactor_word = BN_get_word(cofactor)) == BN_MASK2) > + if ((cofactor_word = BN_get_word(cofactor)) == (BN_ULONG)-1) > goto err; > if (cofactor_word > INT_MAX) > goto err; > Index: usr.bin/dc/bcode.c > =================================================================== > RCS file: /cvs/src/usr.bin/dc/bcode.c,v > diff -u -p -r1.64 bcode.c > --- usr.bin/dc/bcode.c 6 Dec 2025 06:40:49 -0000 1.64 > +++ usr.bin/dc/bcode.c 6 Dec 2025 07:25:20 -0000 > @@ -25,10 +25,6 @@ > > #include "extern.h" > > -#ifndef BN_MASK2 > -#define BN_MASK2 (BN_ULONG)-1 > -#endif > - > /* #define DEBUGGING */ > > #define MAX_ARRAY_INDEX 2048 > @@ -591,7 +587,7 @@ set_scale(void) > warnx("scale must be a nonnegative number"); > else { > scale = get_ulong(n); > - if (scale != BN_MASK2 && scale <= UINT_MAX) > + if (scale != (BN_ULONG)-1 && scale <= UINT_MAX) > bmachine.scale = (u_int)scale; > else > warnx("scale too large"); > @@ -619,7 +615,7 @@ set_obase(void) > n = pop_number(); > if (n != NULL) { > base = get_ulong(n); > - if (base != BN_MASK2 && base > 1 && base <= UINT_MAX) > + if (base != (BN_ULONG)-1 && base > 1 && base <= UINT_MAX) > bmachine.obase = (u_int)base; > else > warnx("output base must be a number greater than 1"); > @@ -646,7 +642,7 @@ set_ibase(void) > n = pop_number(); > if (n != NULL) { > base = get_ulong(n); > - if (base != BN_MASK2 && 2 <= base && base <= 16) > + if (base != (BN_ULONG)-1 && 2 <= base && base <= 16) > bmachine.ibase = (u_int)base; > else > warnx("input base must be a number between 2 and 16 " > @@ -911,7 +907,7 @@ load_array(void) > idx = get_ulong(inumber); > if (BN_is_negative(inumber->number)) > warnx("negative idx"); > - else if (idx == BN_MASK2 || idx > MAX_ARRAY_INDEX) > + else if (idx == (BN_ULONG)-1 || idx > MAX_ARRAY_INDEX) > warnx("idx too big"); > else { > stack = &bmachine.reg[reg]; > @@ -950,7 +946,7 @@ store_array(void) > if (BN_is_negative(inumber->number)) { > warnx("negative idx"); > stack_free_value(value); > - } else if (idx == BN_MASK2 || idx > MAX_ARRAY_INDEX) { > + } else if (idx == (BN_ULONG)-1 || idx > MAX_ARRAY_INDEX) { > warnx("idx too big"); > stack_free_value(value); > } else { > @@ -1200,7 +1196,7 @@ bexp(void) > b = BN_get_word(p->number); > m = max(a->scale, bmachine.scale); > rscale = a->scale * (u_int)b; > - if (rscale > m || (a->scale > 0 && (b == BN_MASK2 || > + if (rscale > m || (a->scale > 0 && (b == (BN_ULONG)-1 || > b > UINT_MAX))) > rscale = m; > } > @@ -1520,7 +1516,7 @@ quitN(void) > return; > i = get_ulong(n); > free_number(n); > - if (i == BN_MASK2 || i == 0) > + if (i == (BN_ULONG)-1 || i == 0) > warnx("Q command requires a number >= 1"); > else if (bmachine.readsp < i) > warnx("Q command argument exceeded string execution depth"); > @@ -1542,7 +1538,7 @@ skipN(void) > if (n == NULL) > return; > i = get_ulong(n); > - if (i == BN_MASK2) > + if (i == (BN_ULONG)-1) > warnx("J command requires a number >= 0"); > else if (i > 0 && bmachine.readsp < i) > warnx("J command argument exceeded string execution depth"); >