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 <nakayamakenjiro@gmail.com>
> ---
>  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)
>