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

[PATCH 2/3] show-branch: store resolved head in heap buffer

From
Jeff King <peff@peff.net>
Date
Feb 14, 2017, 17:27 UTC
Message-ID
<20170214172745.4tnnu6kv7xru54vy@sigill.intra.peff.net>
In-Reply-To
<20170214172526.hzpm3d3ubd3vjnzr@sigill.intra.peff.net>

We resolve HEAD and copy the result to a fixed-size buffer with memcpy, never checking that it actually fits. This bug dates back to 8098a178b (Add git-symbolic-ref, 2005-09-30). Before that we used readlink(), which took a maximum buffer size.

We can fix this by using resolve_refdup(), which duplicates the buffer on the heap. That also lets us just check for a NULL pointer to see if we have resolved HEAD, and drop the extra head_p variable.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/show-branch.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index e4c488b8c..404c4d09a 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -473,8 +473,7 @@ static void snarf_refs(int head, int remotes)
 static int rev_is_head(char *head, char *name,
 		       unsigned char *head_sha1, unsigned char *sha1)
 {
-	if ((!head[0]) ||
-	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
+	if (!head || (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
 	if (starts_with(head, "refs/heads/"))
 		head += 11;
@@ -620,8 +619,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	int all_heads = 0, all_remotes = 0;
 	int all_mask, all_revs;
 	enum rev_sort_order sort_order = REV_SORT_IN_GRAPH_ORDER;
-	char head[128];
-	const char *head_p;
+	char *head;
 	struct object_id head_oid;
 	int merge_base = 0;
 	int independent = 0;
@@ -786,17 +784,10 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			snarf_refs(all_heads, all_remotes);
 	}
 
-	head_p = resolve_ref_unsafe("HEAD", RESOLVE_REF_READING,
-				    head_oid.hash, NULL);
-	if (head_p) {
-		size_t head_len = strlen(head_p);
-		memcpy(head, head_p, head_len + 1);
-	}
-	else {
-		head[0] = 0;
-	}
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING,
+			      head_oid.hash, NULL);
 
-	if (with_current_branch && head_p) {
+	if (with_current_branch && head) {
 		int has_head = 0;
 		for (i = 0; !has_head && i < ref_name_cnt; i++) {
 			/* We are only interested in adding the branch
-- 
2.12.0.rc1.479.g59880b11e
Previous: Jeff KingNext: Jeff King
Message 4 of 15 in “show-branch: fix crash with long ref name”
  1. show-branch: fix crash with long ref nameChristian Couder, Feb 14, 2017
  2. Jeff KingFeb 14, 2017
  3. 1/3 show-branch: drop head_len variableJeff King, Feb 14, 2017
  4. 2/3 show-branch: store resolved head in heap bufferJeff King, Feb 14, 2017
  5. 3/3 show-branch: use skip_prefix to drop magic numbersJeff King, Feb 14, 2017
  6. Pranit BauvaFeb 14, 2017
  7. Jeff KingFeb 14, 2017
  8. Junio C HamanoFeb 14, 2017
  9. Jeff KingFeb 14, 2017
  10. Christian CouderFeb 14, 2017
  11. Jeff KingFeb 15, 2017
  12. Junio C HamanoFeb 15, 2017
  13. Jeff KingFeb 15, 2017
  14. Christian CouderFeb 16, 2017
  15. Jeff KingFeb 17, 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.