Thread (8 messages) flat view 8 messages, 5 authors, 2022-09-02

Re: [PATCH] rev-parse: Detect missing opt-spec

From: SZEDER Gábor <hidden>
Date: 2022-09-02 06:47:36

On Fri, Sep 02, 2022 at 07:06:21AM +0200, Øystein Walle wrote:
If a line in parseopts's input starts with one of the flag characters it
is erroneously parsed as a opt-spec where the short name of the option
is the flag character itself and the long name is after the end of the
string. This makes Git want to allocate SIZE_MAX bytes of memory at this
line:

    o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2);

Since s and sb.buf are equal the second argument is -2 (except unsigned)
and xmemdupz allocates len + 1 bytes, ie. -1 meaning SIZE_MAX.
I suspect (but didn't actually check) that this bug was added in
2d893dff4c (rev-parse --parseopt: allow [*=?!] in argument hints,
2015-07-14).
quoted hunk ↗ jump to hunk
Avoid this by checking whether a flag character was found in the zeroth
position.

Signed-off-by: Øystein Walle <redacted>
---
 builtin/rev-parse.c           | 3 +++
 t/t1502-rev-parse-parseopt.sh | 9 +++++++++
 2 files changed, 12 insertions(+)
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index b259d8990a..04958cf9a9 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -479,6 +479,9 @@ static int cmd_parseopt(int argc, const char **argv, const char *prefix)
 		if (!s)
 			s = help;
 
+		if (s == sb.buf)
+			die(_("Missing opt-spec before option flags"));
+
 		if (s - sb.buf == 1) /* short option only */
 			o->short_name = *sb.buf;
 		else if (sb.buf[1] != ',') /* long option only */
diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
index 284fe18e72..15bc240027 100755
--- a/t/t1502-rev-parse-parseopt.sh
+++ b/t/t1502-rev-parse-parseopt.sh
@@ -306,6 +306,13 @@ test_expect_success 'test --parseopt help output: "wrapped" options normal "or:"
 	test_cmp expect actual
 '
 
+test_expect_success 'test --parseopt invalid opt-spec' '
+	test_write_lines x -- "=, x" >spec &&
+	echo "fatal: Missing opt-spec before option flags" >expect &&
+	test_must_fail git rev-parse --parseopt -- >out <spec >actual 2>&1 &&
When checking an error message please don't look for it on standard
output; i.e. the redirection at the end should be '2>actual', or
perheps even better '2>err'.
quoted hunk ↗ jump to hunk
+	test_cmp expect actual
+'
+
 test_expect_success 'test --parseopt help output: multi-line blurb after empty line' '
 	sed -e "s/^|//" >spec <<-\EOF &&
 	|cmd [--some-option]
@@ -337,3 +344,5 @@ test_expect_success 'test --parseopt help output: multi-line blurb after empty l
 '
 
 test_done
+
+test_done
-- 
2.34.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help