revparse: fix parsing bug for trailing @

ethomson · ethomson · commit c9d31b711e89 · 2024-01-12T10:42:15.000Z
When parsing a revspec that ends with a trailing `@`, explicitly stop parsing. Introduce a sentinel variable to explicitly stop parsing. Prior to this, we would set `spec` to `HEAD`, but were looping on the value of `spec[pos]`, so we would continue walking the (new) `spec` at offset `pos`, looking for a NUL. This is obviously an out-of-bounds read. Credit to Michael Rodler (@f0rki) and Amazon AWS Security.
diff --git a/src/libgit2/revparse.c b/src/libgit2/revparse.c
@@ -701,6 +701,7 @@ static int revparse(
 	git_object *base_rev = NULL;
 
 	bool should_return_reference = true;
+	bool parsed = false;
 
 	GIT_ASSERT_ARG(object_out);
 	GIT_ASSERT_ARG(reference_out);
@@ -710,7 +711,7 @@ static int revparse(
 	*object_out = NULL;
 	*reference_out = NULL;
 
-	while (spec[pos]) {
+	while (!parsed && spec[pos]) {
 		switch (spec[pos]) {
 		case '^':
 			should_return_reference = false;
@@ -817,6 +818,8 @@ static int revparse(
 				break;
 			} else if (spec[pos+1] == '\0') {
 				spec = "HEAD";
+				identifier_len = 4;
+				parsed = true;
 				break;
 			}
 			/* fall through */
