git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 1/4] cat-file: handle NULL object_context.path

From
Jeff King <peff@peff.net>
Date
Sep 21, 2017, 06:21 UTC
Message-ID
<20170921062140.cyfjv7rfuppqpblh@sigill.intra.peff.net>
In-Reply-To
<20170921062043.la2f3pjsnojirzyw@sigill.intra.peff.net>

Commit dc944b65f1 (get_sha1_with_context: dynamically allocate oc->path, 2017-05-19) changed the rules that callers must follow for seeing if we parsed a path in the object name. The rules switched from "check if the oc.path buffer is empty" to "check if the oc.path pointer is NULL". But that commit forgot to update some sites in cat_one_file(), meaning we might dereference a NULL pointer.

You can see this by making a path-aware request like --textconv without specifying --path, and giving an object name that doesn't have a path in it. Like:

  git cat-file --textconv HEAD
which will reliably segfault.
Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/cat-file.c          | 4 ++--
 t/t8010-cat-file-filters.sh | 5 +++++
 2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4ccbfaac31..1ea25331d3 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -97,7 +97,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		return !has_object_file(&oid);
 
 	case 'w':
-		if (!path[0])
+		if (!path)
 			die("git cat-file --filters %s: <object> must be "
 			    "<sha1:path>", obj_name);
 
@@ -107,7 +107,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		break;
 
 	case 'c':
-		if (!path[0])
+		if (!path)
 			die("git cat-file --textconv %s: <object> must be <sha1:path>",
 			    obj_name);
 
diff --git a/t/t8010-cat-file-filters.sh b/t/t8010-cat-file-filters.sh
index d8242e467e..0f86c19174 100755
--- a/t/t8010-cat-file-filters.sh
+++ b/t/t8010-cat-file-filters.sh
@@ -51,6 +51,11 @@ test_expect_success '--path=<path> complains without --textconv/--filters' '
 	grep "path.*needs.*filters" err
 '
 
+test_expect_success '--textconv/--filters complain without path' '
+	test_must_fail git cat-file --textconv HEAD &&
+	test_must_fail git cat-file --filters HEAD
+'
+
 test_expect_success 'cat-file --textconv --batch works' '
 	sha1=$(git rev-parse -q --verify HEAD:world.txt) &&
 	test_config diff.txt.textconv "tr A-Za-z N-ZA-Mn-za-m <" &&
-- 
2.14.1.1051.g8c9143ec35
Previous: Jeff KingNext: Jonathan Nieder
Message 2 of 7 in “appeasing -Wimplicit-fallthrough”
  1. 0/4 appeasing -Wimplicit-fallthroughJeff King, Sep 21, 2017
  2. 1/4 cat-file: handle NULL object_context.pathJeff King, Sep 21, 2017
  3. Jonathan NiederSep 22, 2017
  4. 2/4 test-line-buffer: simplify command parsingJeff King, Sep 21, 2017
  5. Junio C HamanoSep 22, 2017
  6. 3/4 curl_trace(): eliminate switch fallthroughJeff King, Sep 21, 2017
  7. 4/4 consistently use "fallthrough" comments in switchesJeff King, Sep 21, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.