"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
gitconfig: fix read/write out of bound
To:
gameoftrees@openbsd.org
Date:
Sat, 18 Feb 2023 18:47:21 +0100

Download raw body.

Thread
conf_parse_line may read/write out of bounds when a line is indented
by spaces.  This because the loop to trim leading spaces advances the
pointer without adjusting the length of the line.

Since the `line' pointer is part of a bigger buffer, when we try to
"skip trailing whitespace, if any" we're actually writing NUL bytes in
the middle of the next line, mangling it.  (OK, so "out of bounds"
isn't 100% correct, we're not reading/writing arbitrary stuff in the
heap, as long as that part is not triggered on the last line...)

There's also a new regress for this.  The write out of bounds for the
"name" line would mangle the next line, making us skip the author info
from the gitconfig file (as reported by falsifian on IRC.)

(this doesn't affect got directly because gitconfig.c is only included
in the got-read-gitconfig libexec helper ;-)

diff /home/op/w/gotd
commit - cd0aa8caa26478b2cb3c60e889894de02e0eb921
path + /home/op/w/gotd
blob - 4b5de6124b792fc8fcfae5babf81e657a387f457
file + lib/gitconfig.c
--- lib/gitconfig.c
+++ lib/gitconfig.c
@@ -243,8 +243,10 @@ conf_parse_line(char **section, struct got_gitconfig *
 			return got_error_from_errno("strndup");
 		return NULL;
 	}
-	while (isspace((unsigned char)*line))
+	while (sz > 0 && isspace((unsigned char)*line)) {
 		line++;
+		sz--;
+	}
 
 	/* Deal with assignments.  */
 	for (i = 0; i < sz; i++)
blob - 488cdaf04a738a2e6a11df99d0d5df6b7a604c83
file + regress/cmdline/commit.sh
--- regress/cmdline/commit.sh
+++ regress/cmdline/commit.sh
@@ -976,7 +976,40 @@ test_commit_gitconfig_author() {
 	ret=$?
 	if [ $ret -ne 0 ]; then
 		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
 	fi
+
+	# retry with spaces in the git config
+	ed -s "$testroot/repo/.git/config" <<EOF
+,s/	/    /g
+wq
+EOF
+	echo "modified again" > $testroot/wt/alpha
+
+	# unset in a subshell to avoid affecting our environment
+	(unset GOT_IGNORE_GITCONFIG && cd "$testroot/wt" && \
+		got commit -m 'test gitconfig author again' >/dev/null)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	(cd "$testroot/repo" && got log -l1 | grep ^from: > $testroot/stdout)
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	echo "from: Flank Luck <flan_luck@openbsd.org>" \
+		> $testroot/stdout.expectd
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+	fi
 	test_done "$testroot" "$ret"
 }