From: Theo Buehler Subject: Re: [PATCH] LibreSSL: move INT_MAX check before memory allocation in asn1_item_sign() To: Kenjiro Nakayama Cc: tech@openbsd.org Date: Mon, 14 Apr 2025 09:29:52 +0200 On Mon, Apr 14, 2025 at 01:37:17PM +0900, Kenjiro Nakayama wrote: > Reordered the out_len > INT_MAX check in asn1_item_sign() to occur > before calling calloc(), ensuring that big size allocations are > avoided earlier. > > This change has no functional change in logic but only improved > ordering of error handling for efficiency. The reason we check right before ASN1_STRING_set0(..., int len) is that out_len is truncated to an there. So I would like to keep a check there. We could add a second check where you suggest it, but I think it is overdoing it. I don't think this check can realistically be hit - it means that we deal with ASN.1 item 'asn' that is somewhere in the vicinity of 2GB already. Did you actually hit this or is this from code reading? > > Signed-off-by: Kenjiro Nakayama > --- > src/lib/libcrypto/asn1/asn1_item.c | 9 ++++----- > 1 file changed, 4 insertions(+), 5 deletions(-) > > diff --git src/lib/libcrypto/asn1/asn1_item.c src/lib/libcrypto/asn1/asn1_item.c > index 86c800e3a..c9176e65c 100644 > --- src/lib/libcrypto/asn1/asn1_item.c > +++ src/lib/libcrypto/asn1/asn1_item.c > @@ -298,6 +298,10 @@ asn1_item_sign(EVP_MD_CTX *ctx, const ASN1_ITEM *it, void *asn, > ASN1error(ERR_R_EVP_LIB); > goto err; > } > + if (out_len > INT_MAX) { > + ASN1error(ASN1_R_TOO_LONG); > + goto err; > + } > if ((out = calloc(1, out_len)) == NULL) { > ASN1error(ERR_R_MALLOC_FAILURE); > goto err; > @@ -307,11 +311,6 @@ asn1_item_sign(EVP_MD_CTX *ctx, const ASN1_ITEM *it, void *asn, > goto err; > } > > - if (out_len > INT_MAX) { > - ASN1error(ASN1_R_TOO_LONG); > - goto err; > - } > - > ASN1_STRING_set0(signature, out, out_len); > out = NULL; > > -- > 2.39.5 (Apple Git-154) >