Re: [v2 PATCH] builtin: Fix octal escapes in dollar-single-quotes

On Sun, 31 May 2026, at 6:36 AM, Herbert Xu wrote:
> Kerin Millar <[email protected]> wrote:
>> 
>> The test suite of the gentoo-functions project recently uncovered a bug whereby octal sequences within dollar-single-quotes are evaluated as internal control characters, resulting in faulty decoding at best, and segmentation faults at worse. The attached patch rectifies this bug.
>
> Thanks for the patch.
>
> While looking at this I found out that the logic for \x is broken
> as it always produces a multi-byte character even for the value of
> \x81.
>
> So here is a rolled-up patch which fixes both problems.
>
> ---8<---
> From: Kerin Millar <[email protected]>
>
> The test suite of gentoo-functions recently uncovered a bug concerning
> the handling of octal escape sequences within dollar-single-quotes.
> Here is a reproducer:
>
>     $ dash -c "x=\$'\\201'; printf '%s' \"\$x\"" | od -An -tx1
>     88
>
> That is, despite the input being 0x81, the output is 0x88. Indeed, any
> input between 0x81..0x88 is adversely affected and some - such as 0x82 -
> induce a segfault.
>
> I noticed the following macros in src/parser.h:
>
>     #define CTL_FIRST -127 /* first 'special' character */
>     #define CTLESC    -127 /* escape next character */
>     #define CTL_LAST  -120 /* last 'special' character */
>
> Reinterpreted as unsigned char, the CTL_FIRST..CTL_LAST range maps
> exactly to 0x81..0x88:
>
>     $ perl -e 'printf "%x\0", $_ & 0xFF for -127..-120' | xargs -0
>     81 82 83 84 85 86 87 88
>
> From this, I was able to deduce that all of these bytes must be preceded
> by CTLESC in order to be taken literally. Make it so.
>
> Link: 
> https://gitweb.gentoo.org/proj/gentoo-functions.git/commit/?id=947090fb7704
> Signed-off-by: Kerin Millar <[email protected]>
>
> The escape should also be applied to \x sequences.  In fact \x81
> was totally broken as it produced a multi-byte character.  Fix this
> by merging these two code paths.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/src/bltin/printf.c b/src/bltin/printf.c
> index 106aecd..671e781 100644
> --- a/src/bltin/printf.c
> +++ b/src/bltin/printf.c
> @@ -332,6 +332,7 @@ unsigned conv_escape(char *str0, char *out0, bool mbchar)
>  	char *out = out0;
>  	char *str = str0;
>  	unsigned value;
> +	int och;
>  	int ch;
> 
>  	ch = *str;
> @@ -359,12 +360,18 @@ unsigned conv_escape(char *str0, char *out0, bool mbchar)
>  		}
> 
>  		str--;
> +
> +check_value:
> +		if (mbchar && (signed char)value >= CTL_FIRST &&
> +		    (signed char)value <= CTL_LAST)
> +			USTPUTC(CTLESC, out);
>  		break;
> 
>  	case 'x':
>  		ch = 2;
> 
>  hex:
> +		och = ch;
>  		value = 0;
>  		do {
>  			int c = *++str;
> @@ -391,6 +398,9 @@ hex:
>  		if (value < 0x80)
>  			break;
> 
> +		if (och <= 2)
> +			goto check_value;
> +
>  		if (value < 0x110000) {
>  			int mboff = (mbchar - 1) * 2;
>  			unsigned uni = value;

Thank you, that looks great. It occurs to me that I should be able to incorporate some sort of test case for this because, without the revised patch, this happens:

$ printf %s $'\x81' | wc -c
2

-- 
Kerin Millar