Don't print small, negative serial numbers in decimal. X509_print_ex tries to print negative serial numbers in decimal. In doing so, it ends up passing a signed long to %lx and trips -Wformat-signed. A minimal fix would be to cast to unsigned long, but this unsigned long is the absolute value of a signed long (l = -l). This is tricky because -LONG_MIN does not fit in long. It all works because the length check only allows one bit short of sizeof(long)*8 bits (ASN1_INTEGER is sign-and-magnitude). Still, this is a whole lot of subtlety to account for an invalid case. Instead, send negative serial numbers down the generic path. Bug: 450 Change-Id: Ib215fd23863de27e01f7ededf95578f9c800da37 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/50766 Reviewed-by: Adam Langley <agl@google.com> Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c index 7c32a87..486c4ec 100644 --- a/crypto/x509/t_x509.c +++ b/crypto/x509/t_x509.c
@@ -54,6 +54,8 @@ * copied and put under another distribution licence * [including the GNU Public Licence.] */ +#include <assert.h> + #include <openssl/asn1.h> #include <openssl/bio.h> #include <openssl/digest.h> @@ -98,7 +100,6 @@ char *m = NULL, mlch = ' '; int nmindent = 0; X509_CINF *ci; - ASN1_INTEGER *bs; EVP_PKEY *pkey = NULL; const char *neg; @@ -123,33 +124,32 @@ goto err; } if (!(cflag & X509_FLAG_NO_SERIAL)) { - - if (BIO_write(bp, " Serial Number:", 22) <= 0) + if (BIO_write(bp, " Serial Number:", 22) <= 0) { goto err; - - bs = X509_get_serialNumber(x); - if (bs->length < (int)sizeof(long) - || (bs->length == sizeof(long) && (bs->data[0] & 0x80) == 0)) { - l = ASN1_INTEGER_get(bs); - if (bs->type == V_ASN1_NEG_INTEGER) { - l = -l; - neg = "-"; - } else - neg = ""; - if (BIO_printf(bp, " %s%lu (%s0x%lx)\n", neg, l, neg, l) <= 0) - goto err; - } else { - neg = (bs->type == V_ASN1_NEG_INTEGER) ? " (Negative)" : ""; - if (BIO_printf(bp, "\n%12s%s", "", neg) <= 0) - goto err; - - for (i = 0; i < bs->length; i++) { - if (BIO_printf(bp, "%02x%c", bs->data[i], - ((i + 1 == bs->length) ? '\n' : ':')) <= 0) - goto err; - } } + const ASN1_INTEGER *serial = X509_get0_serialNumber(x); + /* |ASN1_INTEGER_get| returns -1 on overflow, so this check skips + * negative and large serial numbers. */ + l = ASN1_INTEGER_get(serial); + if (l >= 0) { + assert(serial->type != V_ASN1_NEG_INTEGER); + if (BIO_printf(bp, " %ld (0x%lx)\n", l, (unsigned long)l) <= 0) { + goto err; + } + } else { + neg = (serial->type == V_ASN1_NEG_INTEGER) ? " (Negative)" : ""; + if (BIO_printf(bp, "\n%12s%s", "", neg) <= 0) { + goto err; + } + + for (i = 0; i < serial->length; i++) { + if (BIO_printf(bp, "%02x%c", serial->data[i], + ((i + 1 == serial->length) ? '\n' : ':')) <= 0) { + goto err; + } + } + } } if (!(cflag & X509_FLAG_NO_SIGNAME)) {