Fix theoretical overflow in ASN1_INTEGER_cmp. While unlikely, ASN1_STRING_cmp is allowed to return INT_MIN (by way of memcmp), in which case negating would overflow. Change-Id: Iec63a6acfad2c662493d22a0acea39ca630881c8 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/51630 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/asn1/a_int.c b/crypto/asn1/a_int.c index 1695fd0..c94f574 100644 --- a/crypto/asn1/a_int.c +++ b/crypto/asn1/a_int.c
@@ -72,22 +72,26 @@ int ASN1_INTEGER_cmp(const ASN1_INTEGER *x, const ASN1_INTEGER *y) { - int neg, ret; - /* Compare signs */ - neg = x->type & V_ASN1_NEG; + /* Compare signs. */ + int neg = x->type & V_ASN1_NEG; if (neg != (y->type & V_ASN1_NEG)) { - if (neg) - return -1; - else - return 1; + return neg ? -1 : 1; } - ret = ASN1_STRING_cmp(x, y); + int ret = ASN1_STRING_cmp(x, y); + if (neg) { + /* This could be |-ret|, but |ASN1_STRING_cmp| is not forbidden from + * returning |INT_MIN|. */ + if (ret < 0) { + return 1; + } else if (ret > 0) { + return -1; + } else { + return 0; + } + } - if (neg) - return -ret; - else - return ret; + return ret; } /*