Skip to content

Commit 5637d55

Browse files
shejialuogitster
authored andcommitted
packed-backend: check whether the refname contains NUL characters
"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. However, it is reported in [1], we cannot catch some corruption. But 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 NUL characters 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 first occurred NUL character is valid. In order to catch above corruption, create a new function "refname_contains_nul" by searching the first NUL character. If it is not at the end of the string, there must be some NUL characters in the refname. Use this function in "next_record" function to die the program if "refname_contains_nul" 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 c92e7e1 commit 5637d55

File tree

1 file changed

+18
-0
lines changed

1 file changed

+18
-0
lines changed

refs/packed-backend.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,21 @@ 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 NUL characters. 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 NUL characters.
506+
*/
507+
static int refname_contains_nul(struct strbuf *refname)
508+
{
509+
return !!memchr(refname->buf, '\0', refname->len);
510+
}
511+
497512
#define SMALL_FILE_SIZE (32*1024)
498513

499514
/*
@@ -895,6 +910,9 @@ static int next_record(struct packed_ref_iterator *iter)
895910
strbuf_add(&iter->refname_buf, p, eol - p);
896911
iter->base.refname = iter->refname_buf.buf;
897912

913+
if (refname_contains_nul(&iter->refname_buf))
914+
die("packed refname contains embedded NULL: %s", iter->base.refname);
915+
898916
if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) {
899917
if (!refname_is_safe(iter->base.refname))
900918
die("packed refname is dangerous: %s",

0 commit comments

Comments
 (0)