From cf7b13f3c30595e3fe88103f9bfb653655a41177 Mon Sep 17 00:00:00 2001 From: Brodie Rao Date: Thu, 11 Aug 2011 14:05:55 -0700 Subject: [PATCH] tag: avoid a double-free when parsing tags without a tagger field The v0.99 tag in the Git repo triggers this behavior: http://git.kernel.org/?p=git/git.git;a=tag;h=d6602ec5194c87b0fc87103ca4d67251c76f233a Ideally, we'd allow the tag to be instantiated even though the tagger field is missing, but this at the very least prevents libgit2 from crashing. To test this bug, a new repository has been added based on the test branch in testrepo.git. It contains a "e90810b" tag that looks like this: object e90810b8df3e80c413d903f631643c716887138d type commit tag e90810b This is a very simple tag. --- src/tag.c | 5 +---- tests/resources/bad_tag.git/HEAD | Bin 0 -> 23 bytes tests/resources/bad_tag.git/config | Bin 0 -> 119 bytes ...28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx | Bin 0 -> 1268 bytes ...8f4e000a17f49a41d7a79fc2f762a8a7d9164.pack | Bin 0 -> 596 bytes tests/resources/bad_tag.git/packed-refs | Bin 0 -> 127 bytes tests/t08-tag.c | 18 ++++++++++++++++++ 7 files changed, 19 insertions(+), 4 deletions(-) create mode 100644 tests/resources/bad_tag.git/HEAD create mode 100644 tests/resources/bad_tag.git/config create mode 100644 tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx create mode 100644 tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack create mode 100644 tests/resources/bad_tag.git/packed-refs diff --git a/src/tag.c b/src/tag.c index fc9f8b5e4..08ebb71cb 100644 --- a/src/tag.c +++ b/src/tag.c @@ -126,11 +126,8 @@ static int parse_tag_buffer(git_tag *tag, const char *buffer, const char *buffer if (tag->tagger == NULL) return GIT_ENOMEM; - if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) != 0) { - free(tag->tag_name); - git_signature_free(tag->tagger); + if ((error = git_signature__parse(tag->tagger, &buffer, buffer_end, "tagger ", '\n')) != 0) return git__rethrow(error, "Failed to parse tag"); - } if( *buffer != '\n' ) return git__throw(GIT_EOBJCORRUPTED, "Failed to parse tag. No new line before message"); diff --git a/tests/resources/bad_tag.git/HEAD b/tests/resources/bad_tag.git/HEAD new file mode 100644 index 0000000000000000000000000000000000000000..cb089cd89a7d7686d284d8761201649346b5aa1c GIT binary patch literal 23 ecmXR)O|w!cN=+-)&qz&7Db~+TEG|hc;sO9;xClW2 literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/config b/tests/resources/bad_tag.git/config new file mode 100644 index 0000000000000000000000000000000000000000..2f8958058adf2a043db52a721e66a626658814a1 GIT binary patch literal 119 zcmaz}&M!)h?o#qRO=VqTIxivecsD%=|nBTLlBSnzYQE)ZF}(RG?r< gQE4h%JSnjVLo6phJuxSzC^fCLASJORwHT%c08(KibN~PV literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx b/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.idx new file mode 100644 index 0000000000000000000000000000000000000000..c404aa15bb04cf5b4fbfc8bb4d9638cd5d420643 GIT binary patch literal 1268 zcmexg;-AdGz`z8=Fu(|8jAF{d02H2-U}m6xVlWF(-6*Ck3|N6-I8-nj(5=Xr9muB& zW=a#Y@_f{Ax{fgShl8jc9xWd3HDVpEvWF5LTe<(DX}8CMQgab9b@(VF?oZ>4zX8h_h;G0T14 z|BH0+oRA_vW%fS@wRi2Ms}zD$t^>2>HXv>VW-mpc-tVk0A!ABL;wH) literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack b/tests/resources/bad_tag.git/objects/pack/pack-7a28f4e000a17f49a41d7a79fc2f762a8a7d9164.pack new file mode 100644 index 0000000000000000000000000000000000000000..90eac50322033985e5623ba611255e53895e2d58 GIT binary patch literal 596 zcmWG=boORoU|<4b_6d9ybLO6LB2ZL-fLMnU}tes`9ibm`tURpR~n zN4$6HD)yU)CWLQZoTPkI!7rVgzg+9v``Z_ETFHpbhKWQbKFLPm4J!}4hZ-;;1 za#iqhJ1^nhV_bNmPxbulr)SmY-soaD~5%da!2C_U9Y#XivXB zA!=`fYW_quaW@M^#aa1lT|@sZc&3)$dY@sb~&ytJ0pw-~IMyXRO#PXcU)}Br5Fo$U@q0+Q~i{wM|OG8(lK~ z2Jxh8?sP1EZO^0=z3#`;4;ObQpM7d2VsCVSdxM_%Er7piGjqipZckmklV`lcJwgzA9)3El zAFkma>fz(b*d@-qv^Yq#$mnyBs8LXN&`ySH`hq*KTjgPveR5ybqU$e~U%h;5+LmYO uLDBtE3=FJx{CBo8;;@Q|AvB%6%_2xXV*ZRJb+SL>o2Gb(o_c6~o*4lB%?J|! literal 0 HcmV?d00001 diff --git a/tests/resources/bad_tag.git/packed-refs b/tests/resources/bad_tag.git/packed-refs new file mode 100644 index 0000000000000000000000000000000000000000..f9fd2fd4ad59d2a735fe4fada3c4b584ae010b30 GIT binary patch literal 127 zcmXZTK?(vf3;@7;UlH&uo0zmse2fsAR7DSqyWsal@HUq@!0O|9eCOQY^VsiaSTkQ4 zP%_-6R6n_C$e`|M(Ud~9Hk&T#M!i<}-DUUNwxppFUd!bVjmfDvgg6X&Hl`*#IyKc! GtWmyyrz5=p literal 0 HcmV?d00001 diff --git a/tests/t08-tag.c b/tests/t08-tag.c index b0d4af87b..079a9e0a8 100644 --- a/tests/t08-tag.c +++ b/tests/t08-tag.c @@ -30,6 +30,7 @@ static const char *tag1_id = "b25fa35b38051e4ae45d4222e795f9df2e43f1d1"; static const char *tag2_id = "7b4384978d2493e851f9cca7858815fac9b10980"; static const char *tagged_commit = "e90810b8df3e80c413d903f631643c716887138d"; +static const char *bad_tag_id = "eda9f45a2a98d4c17a09d681d88569fa4ea91755"; BEGIN_TEST(read0, "read and parse a tag from the repository") git_repository *repo; @@ -106,6 +107,22 @@ BEGIN_TEST(read2, "list all tag names from the repository matching a specified p git_repository_free(repo); END_TEST +#define BAD_TAG_REPOSITORY_FOLDER TEST_RESOURCES "/bad_tag.git/" + +BEGIN_TEST(read3, "read and parse a tag without a tagger field") + git_repository *repo; + git_tag *bad_tag; + git_oid id; + + must_pass(git_repository_open(&repo, BAD_TAG_REPOSITORY_FOLDER)); + + git_oid_fromstr(&id, bad_tag_id); + + must_fail(git_tag_lookup(&bad_tag, repo, &id)); + + git_repository_free(repo); +END_TEST + #define TAGGER_NAME "Vicent Marti" #define TAGGER_EMAIL "vicent@github.com" @@ -304,6 +321,7 @@ BEGIN_SUITE(tag) ADD_TEST(read0); ADD_TEST(read1); ADD_TEST(read2); + ADD_TEST(read3); ADD_TEST(write0); ADD_TEST(write2);