diff options
author | Theo Buehler <tb@openbsd.org> | 2021-05-01 12:25:50 +0200 |
---|---|---|
committer | Tomas Mraz <tomas@openssl.org> | 2021-05-17 10:41:27 +0200 |
commit | c054a6d45dab1bbe38e1c89518e40ba9a9660baa (patch) | |
tree | b3e972447287b9bd855b6d5df825fc72889a64f0 /crypto | |
parent | b743b16113ca0e30c383191c804de37dbfc4f12e (diff) |
Avoid division by zero in hybrid point encoding
In hybrid and compressed point encodings, the form octet contains a bit
of information allowing to calculate y from x. For a point on a binary
curve, this bit is zero if x is zero, otherwise it must match the
rightmost bit of of the field element y / x. The existing code only
considers the second possibility. It could thus incorrecly fail with a
division by zero error as found by Guido Vranken's cryptofuzz.
This commit adds a few explanatory comments to oct2point. The only
actual code change is in the last hunk which adds a BN_is_zero(x)
check to avoid the division by zero.
Fixes #15021
Reviewed-by: Nicola Tuveri <nic.tuv@gmail.com>
Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15112)
Diffstat (limited to 'crypto')
-rw-r--r-- | crypto/ec/ec2_oct.c | 41 |
1 files changed, 33 insertions, 8 deletions
diff --git a/crypto/ec/ec2_oct.c b/crypto/ec/ec2_oct.c index 48543265ee..a0ff0496b3 100644 --- a/crypto/ec/ec2_oct.c +++ b/crypto/ec/ec2_oct.c @@ -247,9 +247,21 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_BUFFER_TOO_SMALL); return 0; } - form = buf[0]; - y_bit = form & 1; - form = form & ~1U; + + /* + * The first octet is the point converison octet PC, see X9.62, page 4 + * and section 4.4.2. It must be: + * 0x00 for the point at infinity + * 0x02 or 0x03 for compressed form + * 0x04 for uncompressed form + * 0x06 or 0x07 for hybrid form. + * For compressed or hybrid forms, we store the last bit of buf[0] as + * y_bit and clear it from buf[0] so as to obtain a POINT_CONVERSION_*. + * We error if buf[0] contains any but the above values. + */ + y_bit = buf[0] & 1; + form = buf[0] & ~1U; + if ((form != 0) && (form != POINT_CONVERSION_COMPRESSED) && (form != POINT_CONVERSION_UNCOMPRESSED) && (form != POINT_CONVERSION_HYBRID)) { @@ -261,6 +273,7 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, return 0; } + /* The point at infinity is represented by a single zero octet. */ if (form == 0) { if (len != 1) { ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_INVALID_ENCODING); @@ -312,11 +325,23 @@ int ec_GF2m_simple_oct2point(const EC_GROUP *group, EC_POINT *point, goto err; } if (form == POINT_CONVERSION_HYBRID) { - if (!group->meth->field_div(group, yxi, y, x, ctx)) - goto err; - if (y_bit != BN_is_odd(yxi)) { - ECerr(EC_F_EC_GF2M_SIMPLE_OCT2POINT, EC_R_INVALID_ENCODING); - goto err; + /* + * Check that the form in the encoding was set correctly + * according to X9.62 4.4.2.a, 4(c), see also first paragraph + * of X9.62, 4.4.1.b. + */ + if (BN_is_zero(x)) { + if (y_bit != 0) { + ECerr(ERR_LIB_EC, EC_R_INVALID_ENCODING); + goto err; + } + } else { + if (!group->meth->field_div(group, yxi, y, x, ctx)) + goto err; + if (y_bit != BN_is_odd(yxi)) { + ECerr(ERR_LIB_EC, EC_R_INVALID_ENCODING); + goto err; + } } } |