[v3,0/3] Disallow NUL character in mailinfo

Message

Đoàn Trần Công Danh April 20, 2020, 11:54 p.m. UTC

As of current state, Git disallow NUL character in commit message.

This indirectly disallow NUL in the body of commit message that sent by email
in UTF-8 encoding.

In those below cases:

* NUL character in header field (be it Subject, Author, Email, etc...)
* NUL in the body of commit message that originally sent in other than UTF-8
  encoding

Git silently accepts them, albeit, truncate at the NUL character.

Those email is clearly not generated by Git, they must be crafted.

Complains loudly about those NUL characters.

Change from v2:

* reword 2/3 commit message
* rename helper for crafting faulty patch, use single heredoc,
  and allow quoted NUL in both subject and body
* postpone a grep in test for NUL in subject to clearly indicate that git-am
  is failing instead of wrong error message.
* quote the header that contains NUL in the error message.


Đoàn Trần Công Danh (3):
  t4254: merge 2 steps of a single test
  mailinfo.c: avoid strlen on strings that can contains NUL
  mailinfo: disallow NUL character in mail's header

 mailinfo.c            | 11 +++++++--
 t/t4254-am-corrupt.sh | 53 +++++++++++++++++++++++++++++++++++++++----
 2 files changed, 58 insertions(+), 6 deletions(-)

Range-diff against v2:
1:  d1bc31e692 = 1:  d1bc31e692 t4254: merge 2 steps of a single test
2:  e3e542d388 ! 2:  97ede4aab2 mailinfo.c::convert_to_utf8: reuse strlen info
    @@ Metadata
     Author: Đoàn Trần Công Danh <[e-mail]>
     
      ## Commit message ##
    -    mailinfo.c::convert_to_utf8: reuse strlen info
    +    mailinfo.c: avoid strlen on strings that can contains NUL
     
         We're passing buffer from strbuf to reencode_string,
         which will call strlen(3) on that buffer,
    @@ t/t4254-am-corrupt.sh
      test_description='git am with corrupt input'
      . ./test-lib.sh
      
    -+write_nul_patch() {
    ++make_mbox_with_nul () {
     +	space=' '
    -+	qNUL=
    -+	case "$1" in
    -+		subject) qNUL='=00' ;;
    -+	esac
    ++	q_nul_in_subject=
    ++	q_nul_in_body=
    ++	while test $# -ne 0
    ++	do
    ++		case "$1" in
    ++		subject) q_nul_in_subject='=00' ;;
    ++		body)    q_nul_in_body='=00' ;;
    ++		esac &&
    ++		shift
    ++	done &&
     +	cat <<-EOF
     +	From ec7364544f690c560304f5a5de9428ea3b978b26 Mon Sep 17 00:00:00 2001
     +	From: A U Thor <[e-mail]>
     +	Date: Sun, 19 Apr 2020 13:42:07 +0700
    -+	Subject: [PATCH] =?ISO-8859-1?q?=C4=CB${qNUL}=D1=CF=D6?=
    ++	Subject: [PATCH] =?ISO-8859-1?q?=C4=CB${q_nul_in_subject}=D1=CF=D6?=
     +	MIME-Version: 1.0
     +	Content-Type: text/plain; charset=ISO-8859-1
    -+	Content-Transfer-Encoding: 8bit
    ++	Content-Transfer-Encoding: quoted-printable
     +
    -+	EOF
    -+	if test "$1" = body
    -+	then
    -+		printf "%s\0%s\n" abc def
    -+	fi
    -+	cat <<-\EOF
    ++	abc${q_nul_in_body}def
     +	---
     +	diff --git a/afile b/afile
     +	new file mode 100644
    @@ t/t4254-am-corrupt.sh: test_expect_success 'try to apply corrupted patch' '
      
     +test_expect_success "NUL in commit message's body" '
     +	test_when_finished "git am --abort" &&
    -+	write_nul_patch body >body.patch &&
    ++	make_mbox_with_nul body >body.patch &&
     +	test_must_fail git am body.patch 2>err &&
     +	grep "a NUL byte in commit log message not allowed" err
     +'
     +
    -+test_expect_failure "NUL in commit message's header" '
    -+	test_when_finished "git am --abort" &&
    -+	write_nul_patch subject >subject.patch &&
    -+	test_must_fail git am subject.patch 2>err &&
    -+	grep "a NUL byte in Subject is not allowed" err
    -+'
    ++test_expect_failure "NUL in commit message's header" "
    ++	test_when_finished 'git am --abort' &&
    ++	make_mbox_with_nul subject >subject.patch &&
    ++	test_must_fail git am subject.patch
    ++"
     +
      test_done
3:  cb96947b36 ! 3:  b2e760227e mailinfo: disallow NUL character in mail's header
    @@ mailinfo.c: static void handle_info(struct mailinfo *mi)
      			continue;
      
     +		if (memchr(hdr->buf, '\0', hdr->len)) {
    -+			error("a NUL byte in %s is not allowed.", header[i]);
    ++			error("a NUL byte in '%s' is not allowed.", header[i]);
     +			mi->input_error = -1;
     +		}
     +
    @@ t/t4254-am-corrupt.sh: test_expect_success "NUL in commit message's body" '
      	grep "a NUL byte in commit log message not allowed" err
      '
      
    --test_expect_failure "NUL in commit message's header" '
    -+test_expect_success "NUL in commit message's header" '
    - 	test_when_finished "git am --abort" &&
    - 	write_nul_patch subject >subject.patch &&
    +-test_expect_failure "NUL in commit message's header" "
    ++test_expect_success "NUL in commit message's header" "
    + 	test_when_finished 'git am --abort' &&
    + 	make_mbox_with_nul subject >subject.patch &&
    +-	test_must_fail git am subject.patch
     +	test_must_fail git mailinfo msg patch <subject.patch 2>err &&
    -+	grep "a NUL byte in Subject is not allowed" err &&
    - 	test_must_fail git am subject.patch 2>err &&
    - 	grep "a NUL byte in Subject is not allowed" err
    - '
    ++	grep \"a NUL byte in 'Subject' is not allowed\" err &&
    ++	test_must_fail git am subject.patch 2>err &&
    ++	grep \"a NUL byte in 'Subject' is not allowed\" err
    + "
    + 
    + test_done