[PATCH 3/6] revert: Parse instruction sheet more cautiously
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Aug 11, 2011, 18:51 UTC
- Message-ID
- <1313088705-32222-4-git-send-email-artagnon@gmail.com>
- In-Reply-To
- <1313088705-32222-1-git-send-email-artagnon@gmail.com>
Fix a buffer overflow bug by checking that parsed SHA-1 hex will fit in the buffer we've created for it. Also change the instruction sheet format subtly so that a description of the commit after the object name is optional. So now, an instruction sheet like this is perfectly valid:
pick 35b0426 pick fbd5bbcbc2e pick 7362160f
Suggested-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com> --- builtin/revert.c | 20 +++++++++----------- t/t3510-cherry-pick-sequence.sh | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 11 deletions(-)
diff --git a/builtin/revert.c b/builtin/revert.c index 1a4187a..f44f749 100644 --- a/builtin/revert.c +++ b/builtin/revert.c @@ -697,26 +697,24 @@ static struct commit *parse_insn_line(char *start, struct replay_opts *opts) unsigned char commit_sha1[20]; char sha1_abbrev[40]; enum replay_action action; - int insn_len = 0; - char *p, *q; + char *p = start, *q, *end = strchrnul(start, '\n'); if (!prefixcmp(start, "pick ")) { action = CHERRY_PICK; - insn_len = strlen("pick"); - p = start + insn_len + 1; + p += strlen("pick "); } else if (!prefixcmp(start, "revert ")) { action = REVERT; - insn_len = strlen("revert"); - p = start + insn_len + 1; + p += strlen("revert "); } else return NULL; - q = strchr(p, ' '); - if (!q) + q = strchrnul(p, ' '); + if (q > end) + q = end; + if (q - p + 1 > sizeof(sha1_abbrev)) return NULL; - q++; - - strlcpy(sha1_abbrev, p, q - p); + memcpy(sha1_abbrev, p, q - p); + sha1_abbrev[q - p] = '\0'; /* * Verify that the action matches up with the one in diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh index 3bca2b3..bc5f0b8 100755 --- a/t/t3510-cherry-pick-sequence.sh +++ b/t/t3510-cherry-pick-sequence.sh @@ -211,4 +211,33 @@ test_expect_success 'malformed instruction sheet 2' ' test_must_fail git cherry-pick --continue ' +test_expect_success 'missing commit descriptions in instruction sheet' ' + pristine_detach initial && + test_must_fail git cherry-pick base..anotherpick && + echo "c" >foo && + git add foo && + git commit && + cut -d" " -f1,2 .git/sequencer/todo >new_sheet && + cp new_sheet .git/sequencer/todo && + git cherry-pick --continue && + test_path_is_missing .git/sequencer && + { + git rev-list HEAD | + git diff-tree --root --stdin | + sed "s/$_x40/OBJID/g" + } >actual && + cat >expect <<-\EOF && + OBJID + :100644 100644 OBJID OBJID M foo + OBJID + :100644 100644 OBJID OBJID M foo + OBJID + :100644 100644 OBJID OBJID M unrelated + OBJID + :000000 100644 OBJID OBJID A foo + :000000 100644 OBJID OBJID A unrelated + EOF + test_cmp expect actual +' + test_done
-- 1.7.6.351.gb35ac.dirty