fast-import: duplicate parsed encoding string

We read each line of the fast-import stream into the command_buf strbuf. When reading a commit, we parse a line like "encoding foo" by storing a pointer to "foo", but not making a copy. We may then read an unbounded number of other lines (e.g., one for each modified file in the commit), each of which writes into command_buf. This works out in practice for small cases, because we hand off ownership of the heap buffer from command_buf to the cmd_hist array, and read new commands into a fresh heap buffer. And thus the pointer to "foo" remains valid as long as there aren't so many intermediate lines that we end up dropping the original "encoding" line from the history. But as the test modification shows, if we go over our default of 100 lines, we end up with our encoding string pointing into freed heap memory. This seems to fail reliably by writing garbage into the output, but running under ASan definitely detects this as a use-after-free. We can fix it by duplicating the encoding value, just as we do for other parsed lines (e.g., an author line ends up in parse_ident, which copies it to a new string). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Aug 25, 2019 at 04:08 UTC 9756082b3cfaf69574bc3283cf4c9ba9c91442bc
2 files changed +10 -2
fast-import.c
+5 -2
@@ -2588,7 +2588,7 @@ static void parse_new_commit(const char *arg)
2588 struct branch *b;
2589 char *author = NULL;
2590 char *committer = NULL;
2591 - const char *encoding = NULL;
2591 + char *encoding = NULL;
2592 struct hash_list *merge_list = NULL;
2593 unsigned int merge_count;
2594 unsigned char prev_fanout, new_fanout;
@@ -2611,8 +2611,10 @@ static void parse_new_commit(const char *arg)
2611 }
2612 if (!committer)
2613 die("Expected committer but didn't get one");
2614 - if (skip_prefix(command_buf.buf, "encoding ", &encoding))
2614 + if (skip_prefix(command_buf.buf, "encoding ", &v)) {
2615 + encoding = xstrdup(v);
2616 read_next_command();
2617 + }
2618 parse_data(&msg, 0, NULL);
2619 read_next_command();
2620 parse_from(b);
@@ -2686,6 +2688,7 @@ static void parse_new_commit(const char *arg)
2688 strbuf_addbuf(&new_data, &msg);
2689 free(author);
2690 free(committer);
2691 + free(encoding);
2692
2693 if (!store_object(OBJ_COMMIT, &new_data, NULL, &b->oid, next_mark))
2694 b->pack_id = pack_id;
t/t9300-fast-import.sh
+5
@@ -3314,6 +3314,11 @@ test_expect_success 'X: handling encoding' '
3314
3315 printf "Pi: \360\nCOMMIT\n" >>input &&
3316
3317 + for i in $(test_seq 100)
3318 + do
3319 + echo "M 644 $EMPTY_BLOB file-$i"
3320 + done >>input &&
3321 +
3322 git fast-import <input &&
3323 git cat-file -p encoding | grep $(printf "\360") &&
3324 git log -1 --format=%B encoding | grep $(printf "\317\200")