Skip to content

Commit 1be2de2

Browse files
shejialuogitster
authored andcommitted
packed-backend: check whether the refname contains NULL binaries
We have already implemented the header consistency check for the raw "packed-refs" file. Before we implement the consistency check for each ref entry, let's analysis [1] which reports that "git fsck" cannot detect some binary zeros. "packed-backend.c::next_record" will use "check_refname_format" to check the consistency of the refname. If it is not OK, the program will die. So, we already have the code path and we must miss out something. We use the following code to get the refname: strbuf_add(&iter->refname_buf, p, eol - p); iter->base.refname = iter->refname_buf.buf In the above code, `p` is the start pointer of the refname and `eol` is the next newline pointer. We calculate the length of the refname by subtracting the two pointers. Then we add the memory range between `p` and `eol` to get the refname. However, if there are some NULL binaries in the memory range between `p` and `eol`, we will see the refname as a valid ref name as long as the memory range between `p` and the first occurred NULL binary is valid. In order to catch above corruption, create a new function "refname_contains_null" by checking whether the "refname.len" equals to the length of the raw string pointer "refname.buf". If not equal, there must be some NULL binaries in the refname. Use this function in "next_record" function to die the program if "refname_contains_null" returns true. [1] https://lore.kernel.org/git/[email protected]/ Reported-by: R. Diez <[email protected]> Mentored-by: Patrick Steinhardt <[email protected]> Mentored-by: Karthik Nayak <[email protected]> Signed-off-by: shejialuo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent e859555 commit 1be2de2

File tree

1 file changed

+20
-0
lines changed

1 file changed

+20
-0
lines changed

refs/packed-backend.c

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,23 @@ static void verify_buffer_safe(struct snapshot *snapshot)
494494
last_line, eof - last_line);
495495
}
496496

497+
/*
498+
* When parsing the "packed-refs" file, we will parse it line by line.
499+
* Because we know the start pointer of the refname and the next
500+
* newline pointer, we could calculate the length of the refname by
501+
* subtracting the two pointers. However, there is a corner case where
502+
* the refname contains corrupted embedded NULL binaries. And
503+
* `check_refname_format()` will not catch this when the truncated
504+
* refname is still a valid refname. To prevent this, we need to check
505+
* whether the refname contains the NULL binaries.
506+
*/
507+
static int refname_contains_null(struct strbuf refname)
508+
{
509+
if (refname.len != strlen(refname.buf))
510+
return 1;
511+
return 0;
512+
}
513+
497514
#define SMALL_FILE_SIZE (32*1024)
498515

499516
/*
@@ -895,6 +912,9 @@ static int next_record(struct packed_ref_iterator *iter)
895912
strbuf_add(&iter->refname_buf, p, eol - p);
896913
iter->base.refname = iter->refname_buf.buf;
897914

915+
if (refname_contains_null(iter->refname_buf))
916+
die("packed refname contains embedded NULL: %s", iter->base.refname);
917+
898918
if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) {
899919
if (!refname_is_safe(iter->base.refname))
900920
die("packed refname is dangerous: %s",

0 commit comments

Comments
 (0)