From efb484e86f138b6e503aaecd54723db297a3576d Mon Sep 17 00:00:00 2001 From: tb Date: Tue, 28 Dec 2021 15:49:11 +0000 Subject: [PATCH] Convert X509v3_adr_get_afi() to CBS The manual byte bashing is performed more safely using this API which would have avoided the out-of-bounds read that this API had until a few years back. The API is somewhat strange in that it uses the reserved AFI 0 as an in-band error but it doesn't care about the reserved AFI 65535. ok inoguchi jsing --- lib/libcrypto/x509/x509_addr.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/libcrypto/x509/x509_addr.c b/lib/libcrypto/x509/x509_addr.c index bb2e9e58674..64dd8305148 100644 --- a/lib/libcrypto/x509/x509_addr.c +++ b/lib/libcrypto/x509/x509_addr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: x509_addr.c,v 1.28 2021/12/25 23:35:25 tb Exp $ */ +/* $OpenBSD: x509_addr.c,v 1.29 2021/12/28 15:49:11 tb Exp $ */ /* * Contributed to the OpenSSL Project by the American Registry for * Internet Numbers ("ARIN"). @@ -73,6 +73,7 @@ #include #include +#include "bytestring.h" #include "x509_lcl.h" #ifndef OPENSSL_NO_RFC3779 @@ -330,16 +331,30 @@ length_from_afi(const unsigned afi) /* * Extract the AFI from an IPAddressFamily. + * + * This is public API. It uses the reserved AFI 0 as an in-band error + * while it doesn't care about the reserved AFI 65535... */ unsigned int X509v3_addr_get_afi(const IPAddressFamily *f) { - if (f == NULL || - f->addressFamily == NULL || - f->addressFamily->data == NULL || - f->addressFamily->length < 2) + CBS cbs; + uint16_t afi; + + /* + * XXX are these NULL checks really sensible? If f is non-NULL, it + * should have both addressFamily and ipAddressChoice... + */ + if (f == NULL || f->addressFamily == NULL || + f->addressFamily->data == NULL) return 0; - return (f->addressFamily->data[0] << 8) | f->addressFamily->data[1]; + + CBS_init(&cbs, f->addressFamily->data, f->addressFamily->length); + + if (!CBS_get_u16(&cbs, &afi)) + return 0; + + return afi; } /* -- 2.20.1