Re: [v2 PATCH] expand: Escape minus sign in arithmetic expansion

[Jilles Tjoelker <jilles@xxxxxxxx>]

On Tue, May 29, 2018 at 02:00:26AM +0800, Herbert Xu wrote:
> On Mon, May 28, 2018 at 12:22:00AM +0800, Herbert Xu wrote:
> > The minus sign generated from arithmetic expansion is currently
> > unquoted which causes anomalies when the result is used in where
> > the quoting matters.

> > This patch fixes it by explicitly calling memtodest for the minus
> > sign.

> > Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

> This was buggy.  Here is an update.

> ---8<---
> The minus sign generated from arithmetic expansion is currently
> unquoted which causes anomalies when the result is used in where
> the quoting matters.

> This patch fixes it by explicitly calling memtodest for the minus
> sign.

> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>

> diff --git a/src/expand.c b/src/expand.c
> index 7a51766..7ed1bc0 100644
> --- a/src/expand.c
> +++ b/src/expand.c
> @@ -490,7 +490,14 @@ expari(int flag)
>  	result = arith(p + 1);
>  	popstackmark(&sm);
>  
> -	len = cvtnum(result);
> +	len = 0;
> +	if (result < 0) {
> +		memtodest("-", 1, flag);
> +		result = -result;

This negation overflows if result is INTMAX_MIN, leading to undefined
behaviour. Perhaps leave the memtodest call but use STADJUST to remove
the minus sign itself, leaving only the CTLESC or nothing.

> +		len++;
> +	}
> +
> +	len += cvtnum(result);
>  
>  	if (likely(!(flag & EXP_QUOTED)))
>  		recordregion(begoff, begoff + len, 0);

-- 
Jilles Tjoelker
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html