ref-filter: fix parsing of signatures with CRLF and no body

This commit fixes a bug when parsing tags that have CRLF line endings, a signature, and no body, like this (the "^M" are marking the CRs): this is the subject^M -----BEGIN PGP SIGNATURE-----^M ^M ...some stuff...^M -----END PGP SIGNATURE-----^M When trying to find the start of the body, we look for a blank line separating the subject and body. In this case, there isn't one. But we search for it using strstr(), which will find the blank line in the signature. In the non-CRLF code path, we check whether the line we found is past the start of the signature, and if so, put the body pointer at the start of the signature (effectively making the body empty). But the CRLF code path doesn't catch the same case, and we end up with the body pointer in the middle of the signature field. This has two visible problems: - printing %(contents:subject) will show part of the signature, too, since the subject length is computed as (body - subject) - the length of the body is (sig - body), which makes it negative. Asking for %(contents:body) causes us to cast this to a very large size_t when we feed it to xmemdupz(), which then complains about trying to allocate too much memory. These are essentially the same bugs fixed in the previous commit, except that they happen when there is a CRLF blank line in the signature, rather than no blank line at all. Both are caused by the refactoring in 9f75ce3d8f (ref-filter: handle CRLF at end-of-line more gracefully, 2020-10-29). We can fix this by doing the same "sigstart" check that we do in the non-CRLF case. And rather than repeat ourselves, we can just use short-circuiting OR to collapse both cases into a single conditional. I.e., rather than: if (strstr("\n\n")) ...found blank, check if it's in signature... else if (strstr("\r\n\r\n")) ...found blank, check if it's in signature... else ...no blank line found... we can collapse this to: if (strstr("\n\n")) || strstr("\r\n\r\n"))) ...found blank, check if it's in signature... else ...no blank line found... The tests show the problem and the fix. Though it wasn't broken, I included contents:signature here to make sure it still behaves as expected, but note the shell hackery needed to make it work. A less-clever option would be to skip using test_atom and just "append_cr >expected" ourselves. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Taylor Blau <me@ttaylorr.com>

Jeff King committed Nov 2, 2022 at 03:44 UTC 8e1c5fcf286322c019e376563781458e6a719231
2 files changed +28 -3
ref-filter.c
+3 -3
@@ -1375,10 +1375,10 @@ static void find_subpos(const char *buf,
1375 /* subject is first non-empty line */
1376 *sub = buf;
1377 /* subject goes to first empty line before signature begins */
1378 - if ((eol = strstr(*sub, "\n\n"))) {
1378 + if ((eol = strstr(*sub, "\n\n")) ||
1379 + (eol = strstr(*sub, "\r\n\r\n"))) {
1380 eol = eol < sigstart ? eol : sigstart;
1380 - /* check if message uses CRLF */
1381 - } else if (! (eol = strstr(*sub, "\r\n\r\n"))) {
1381 + } else {
1382 /* treat whole message as subject */
1383 eol = sigstart;
1384 }
t/t6300-for-each-ref.sh
+25
@@ -1421,4 +1421,29 @@ test_atom refs/tags/fake-sig-no-blanks contents:subject 'this is the subject'
1421 test_atom refs/tags/fake-sig-no-blanks contents:body ''
1422 test_atom refs/tags/fake-sig-no-blanks contents:signature "$sig"
1423
1424 +test_expect_success 'set up tag with CRLF signature' '
1425 + append_cr <<-\EOF |
1426 + this is the subject
1427 + -----BEGIN PGP SIGNATURE-----
1428 +
1429 + not a real signature, but we just care about
1430 + the subject/body parsing. It is important here
1431 + that there is a blank line separating this
1432 + from the signature header.
1433 + -----END PGP SIGNATURE-----
1434 + EOF
1435 + git tag -F - --cleanup=verbatim fake-sig-crlf
1436 +'
1437 +
1438 +test_atom refs/tags/fake-sig-crlf contents:subject 'this is the subject'
1439 +test_atom refs/tags/fake-sig-crlf contents:body ''
1440 +
1441 +# CRLF is retained in the signature, so we have to pass our expected value
1442 +# through append_cr. But test_atom requires a shell string, which means command
1443 +# substitution, and the shell will strip trailing newlines from the output of
1444 +# the substitution. Hack around it by adding and then removing a dummy line.
1445 +sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)"
1446 +sig_crlf=${sig_crlf%dummy}
1447 +test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf"
1448 +
1449 test_done