This follows the paranoid spirit in 37576c1 (commit_tree(): refuse
commit messages that contain NULs - 2011-12-15) and stops users from
creating tags that contain NULs. If these tags are merged into a commit
as a mergetag line, it may break the commit header processing badly.
While at it, check for NULs in mergetag and gpgsig commit headers, and
check again at write_sha1_file() as the last resort, mostly to catch
programming errors.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
---
2012/2/20 Junio C Hamano <gitster@xxxxxxxxx>:
>> if (verify_tag(buf.buf, buf.len) < 0)
>> die("invalid tag signature file");
>>
>> + if (memchr(buf.buf, '\0', buf.len))
>> + die("a NUL byte in tag message not allowed.");
>> +
>
> Is there a good reason why you check _after_ calling verify_tag(), instead
> of before?
[...]
>> if (sign && do_sign(buf) < 0)
>> return error(_("unable to sign the tag"));
>> + if (memchr(buf->buf, '\0', buf->len))
>> + return error("a NUL byte in tag message not allowed.");
>
> Is there a good reason why you check _after_ calling do_sign(), instead of
> before?
It's not about after those. It's about right before write_sha1_file().
I wanted to catch all NULs no matter how they come. But yes the check
should happen early to avoid wasting user's time (e.g. doing signing)
So how about this?
builtin/mktag.c | 2 ++
builtin/tag.c | 2 ++
commit.c | 2 ++
sha1_file.c | 6 ++++++
t/t7004-tag.sh | 4 ++++
t/t7510-signed-commit.sh | 2 +-
6 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/builtin/mktag.c b/builtin/mktag.c
index 640ab64..e579471 100644
--- a/builtin/mktag.c
+++ b/builtin/mktag.c
@@ -160,6 +160,8 @@ int cmd_mktag(int argc, const char **argv, const char *prefix)
die_errno("could not read from stdin");
}
+ if (memchr(buf.buf, '\0', buf.len))
+ return error("a NUL byte in tag message not allowed.");
/* Verify it for some basic sanity: it needs to start with
"object <sha1>\ntype\ntagger " */
if (verify_tag(buf.buf, buf.len) < 0)
diff --git a/builtin/tag.c b/builtin/tag.c
index 31f02e8..3e284f7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -257,6 +257,8 @@ static void write_tag_body(int fd, const unsigned char *sha1)
static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
{
+ if (memchr(buf->buf, '\0', buf->len))
+ return error("a NUL byte in tag message not allowed.");
if (sign && do_sign(buf) < 0)
return error(_("unable to sign the tag"));
if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
diff --git a/commit.c b/commit.c
index 4b39c19..545325f 100644
--- a/commit.c
+++ b/commit.c
@@ -1144,6 +1144,8 @@ int commit_tree_extended(const struct strbuf *msg, unsigned char *tree,
strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
while (extra) {
+ if (memchr(extra->value, '\0', extra->len))
+ return error("a NUL byte in commit header %s not allowed.", extra->key);
add_extra_header(&buffer, extra);
extra = extra->next;
}
diff --git a/sha1_file.c b/sha1_file.c
index 88f2151..2fc8623 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2519,6 +2519,12 @@ int write_sha1_file(const void *buf, unsigned long len, const char *type, unsign
char hdr[32];
int hdrlen;
+ /* GIT_HASH_NUL is for the test suite to hash abitrary content */
+ if (!getenv("GIT_HASH_NUL") &&
+ (!strcmp(type, commit_type) || !strcmp(type, tag_type)) &&
+ memchr(buf, '\0', len))
+ return error("BUG: %s message contains NUL.", type);
+
/* Normally if we have it in the pack then we do not bother writing
* it out into .git/objects/??/?{38} file.
*/
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index e93ac73..8cb13e5 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1269,4 +1269,8 @@ test_expect_success 'mixing incompatibles modes and options is forbidden' '
test_must_fail git tag -v -s
'
+test_expect_success 'tag content contains NUL' '
+ test_must_fail git tag -F "$TEST_DIRECTORY"/t3900/UTF-16.txt utf16
+'
+
test_done
diff --git a/t/t7510-signed-commit.sh b/t/t7510-signed-commit.sh
index 1d3c56f..d75a349 100755
--- a/t/t7510-signed-commit.sh
+++ b/t/t7510-signed-commit.sh
@@ -63,7 +63,7 @@ test_expect_success GPG 'detect fudged signature with NUL' '
git cat-file commit master >raw &&
cat raw >forged2 &&
echo Qwik | tr "Q" "\000" >>forged2 &&
- git hash-object -w -t commit forged2 >forged2.commit &&
+ GIT_HASH_NUL=1 git hash-object -w -t commit forged2 >forged2.commit &&
git show --pretty=short --show-signature $(cat forged2.commit) >actual2 &&
grep "BAD signature from" actual2 &&
! grep "Good signature from" actual2
--
1.7.8.36.g69ee2
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html