parser: Do not read past single quote in dollarsq_escape

Commit Message

Herbert Xu July 6, 2024, 1:37 a.m. UTC

наб <[e-mail]> wrote:
> 
> Current HEAD of 776424a8f9158bfe9f53aa55f931af9f73437caf
> ("parser: Add dollar single quote"):
>  $ printf '%s\n' $'\123'
> simply hangs.
> 
> strace shows
>  read(0, printf '%s\n' $'\123'
>  "printf '%s\\n' $'\\123'\n", 8192) = 22
>  read(0,
> 
> Bisecting this says that this is the first problematic commit.
> 
> Actually writing around five bytes makes it write the S\n and continue
> (and interpret those five bytes as-if typed at the prompt).

Thanks for the report.  This patch should fix the problem:

---8<---
The function dollarsq_escape may read past the current escape
code in order to provide enough data to the underlying escape
code processing function.  This is OK because we will call unget
to return any unused characters.  However, if this occurs at
the end of a quoted string, this may prompt the user for more
input which is wrong.

Fix this by terminating the loop whenever we see a single quote.
Even if this is an escaped single quote and thus does not indicate
the end of the whole quoted string, it's still OK because no single
escape code can continue after a single quote.

Reported-by: наб <[e-mail]>
Fixes: 776424a8f915 ("parser: Add dollar single quote")
Signed-off-by: Herbert Xu <[e-mail]>
---
 src/parser.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

наб July 6, 2024, 3:09 a.m. UTC | #1

On Sat, Jul 06, 2024 at 11:37:16AM +1000, Herbert Xu wrote:
> The function dollarsq_escape may read past the current escape
> code in order to provide enough data to the underlying escape
> code processing function.  This is OK because we will call unget
> to return any unused characters.  However, if this occurs at
> the end of a quoted string, this may prompt the user for more
> input which is wrong.
> 
> Fix this by terminating the loop whenever we see a single quote.
> Even if this is an escaped single quote and thus does not indicate
> the end of the whole quoted string, it's still OK because no single
> escape code can continue after a single quote.
Yeah, works for me.

Patch

diff --git a/src/parser.c b/src/parser.c
index d1bec58..aecc18f 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -938,13 +938,16 @@  static char *dollarsq_escape(char *out)
 	unsigned len;
 	char *p;
 
-	for (len = 0; len < sizeof(str) - 1; len++) {
+	for (len = 0; len < sizeof(str) - 1;) {
 		int c = pgetc();
 
 		if (c <= PEOF)
 			break;
 
-		str[len] = c;
+		str[len++] = c;
+
+		if (c == '\'')
+			break;
 	}
 	str[len] = 0;