@matheusbsbs / matheus-bagliano / commits / 5637d55420

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/6cfee0e4-3285-4f18-91ff-d097da9de737@rd10.de/ Reported-by: R. Diez <rdiez-temp3@rd10.de> Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: shejialuo <shejialuo@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

shejialuo committed Feb 28, 2025 at 00:07 UTC 5637d5542021294e81cf0d8344fe140368117296

1 file changed +18

refs/packed-backend.c

+18

@@ -494,6 +494,21 @@ static void verify_buffer_safe(struct snapshot *snapshot)

494 last_line, eof - last_line);

495 }

496

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 +

512 #define SMALL_FILE_SIZE (32*1024)

513

514 /*

@@ -895,6 +910,9 @@ static int next_record(struct packed_ref_iterator *iter)

910 strbuf_add(&iter->refname_buf, p, eol - p);

911 iter->base.refname = iter->refname_buf.buf;

912

913 + if (refname_contains_nul(&iter->refname_buf))

914 + die("packed refname contains embedded NULL: %s", iter->base.refname);

915 +

916 if (check_refname_format(iter->base.refname, REFNAME_ALLOW_ONELEVEL)) {

917 if (!refname_is_safe(iter->base.refname))

918 die("packed refname is dangerous: %s",