{"thread":{"id":"57834","subject":"Bug report - ssh signing causes git tag -l malloc failed","startedAt":"2022-05-02T19:51:41Z","lastAt":"2022-05-03T01:10:02Z","messageCount":3,"participants":["Brad Beam","Junio C Hamano","Taylor Blau"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"454747","messageId":"16668F1B-7670-4136-8AAF-ABAAA802C7D4@b-rad.info","threadId":"57834","inReplyTo":null,"subject":"Bug report - ssh signing causes git tag -l malloc failed","fromName":"Brad Beam","fromEmail":"brad.beam@b-rad.info","sentAt":"2022-05-02T19:51:33Z","receivedAt":"2022-05-02T19:51:41Z","isPatch":false,"sender":{"key":"brad.beam@b-rad.info","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n\nWhen using ssh signing `git tag -l --format='%(contents:body)' <tag>` returns `fatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)`\n\nWhat did you expect to happen? (Expected behavior)\n\nThe tag to be returned without crashing\n\nWhat happened instead? (Actual behavior)\n\ngit crashed with the above error (`fatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)`)\n\nWhat's different between what you expected and what actually happened?\n\nAnything else you want to add:\n\n\n```\n[14:23:54] (0):~/src/github.com/my/repo\n% git tag -sam v0.0.9 v0.0.9\n\n[14:24:03] (0):~/src/github.com/my/repo\n% git -c log.showSignature=false tag -l --format='%(contents:body)' v0.0.9\nfatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)\n\n[14:24:27] (0):~/src/github.com/my/repo\n% git tag -sm v0.0.9-no-annotate v0.0.9-no-annotate\n\n[14:27:19] (0):~/src/github.com/my/repo\n% git -c log.showSignature=false tag -l --format='%(contents:body)' v0.0.9-no-annotate\nfatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)\n\n[14:27:26] (128):~/src/github.com/my/repo\n% git tag -m v0.0.9-no-annotate-no-ssh v0.0.9-no-annotate-no-ssh\n\n[14:28:10] (0):~/src/github.com/my/repo\n% git -c log.showSignature=false tag -l --format='%(contents:body)' v0.0.9-no-annotate-no-ssh\n\n```\n\nPlease review the rest of the bug report below.\nYou can delete any lines you don't wish to share.\n\n\n[System Info]\ngit version:\ngit version 2.36.0\ncpu: x86_64\nno commit associated with this build\nsizeof-long: 8\nsizeof-size_t: 8\nshell-path: /bin/sh\nfeature: fsmonitor--daemon\nuname: Darwin 20.6.0 Darwin Kernel Version 20.6.0: Tue Oct 12 18:33:42 PDT 2021; root:xnu-7195.141.8~1/RELEASE_X86_64 x86_64\ncompiler info: clang: 13.0.0 (clang-1300.0.29.30)\nlibc info: no libc information available\n$SHELL (typically, interactive shell): /bin/zsh\n\n\n[Enabled Hooks]\n\n"},{"id":"454748","messageId":"xmqq7d73y97i.fsf@gitster.g","threadId":"57834","inReplyTo":"16668F1B-7670-4136-8AAF-ABAAA802C7D4@b-rad.info","subject":"Re: Bug report - ssh signing causes git tag -l malloc failed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-02T20:24:01Z","receivedAt":"2022-05-02T20:24:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brad Beam <brad.beam@b-rad.info> writes:\n\nThanks for a report.\n\n\n> What did you do before the bug happened? (Steps to reproduce your issue)\n>\n> When using ssh signing `git tag -l --format='%(contents:body)' <tag>` returns `fatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)`\n\nAn obvious first follow-up question is if there is any difference in\nbehaviour if another kind of signing (like PGP) is used.\n\n> What did you expect to happen? (Expected behavior)\n>\n> The tag to be returned without crashing\n>\n> What happened instead? (Actual behavior)\n>\n> git crashed with the above error (`fatal: Out of memory, malloc failed (tried to allocate 18446744073709551323 bytes)`)\n>\n> What's different between what you expected and what actually happened?\n>\n> Anything else you want to add:\n>\n>\n> ```\n> [14:23:54] (0):~/src/github.com/my/repo\n> % git tag -sam v0.0.9 v0.0.9\n\nHere, or before this step, there would probably have been something\nto say \"No, I do not use the default PGP sign, but I want SSH sign\"?\n\nFor those who are reading this bug report and mistakenly thought it\nis a bug to show a self-recursive tag (like I did during my first\nreading of the report), the first v0.0.9 is merely an argument to\nthe \"-m\" option, and the second v0.0.9 is the name of the newly\ncreated tag.  We are creating a signed tag with v0.0.9 in the\nmessage that points at HEAD and has \"tag v0.0.9\" in the header, and\nstoring the resulting tag at refs/tags/v0.0.9\n\n"},{"id":"454759","messageId":"YnB/Pam2ti1A8c0e@nand.local","threadId":"57834","inReplyTo":"xmqq7d73y97i.fsf@gitster.g","subject":"Re: Bug report - ssh signing causes git tag -l malloc failed","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-05-03T01:02:53Z","receivedAt":"2022-05-03T01:10:02Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, May 02, 2022 at 01:24:01PM -0700, Junio C Hamano wrote:\n> Brad Beam <brad.beam@b-rad.info> writes:\n>\n> Thanks for a report.\n>\n> > What did you do before the bug happened? (Steps to reproduce your\n> > issue)\n> >\n> > When using ssh signing `git tag -l --format='%(contents:body)'\n> > <tag>` returns `fatal: Out of memory, malloc failed (tried to\n> > allocate 18446744073709551323 bytes)`\n>\n> An obvious first follow-up question is if there is any difference in\n> behaviour if another kind of signing (like PGP) is used.\n\nIt works when using gpg.format=openpgp, and breaks reliably when signing\nusing OpenSSH. The simplest reproduction I could come up with is:\n\n    git init repo\n    (\n      cd repo\n      >foo\n      git add foo\n      git commit -m \"$(date)\"\n      git tag -sam foo foo\n    )\n\nI think the difference is that when signing with OpenPGP, the \"foo\" tag\nlooks like this:\n\n    $ git -C repo cat-file -p foo\n    object d9b42c39c8520b2b9d62d67aa4138e1a28ce7aee\n    type commit\n    tag foo\n    tagger Taylor Blau <me@ttaylorr.com> 1651538928 -0400\n\n    foo\n    -----BEGIN PGP SIGNATURE-----\n\n    iQEzBAABCgAdFiEEuSAhSeE5DcWB+EsnUHZwBbY/KHsFAmJwe/AACgkQUHZwBbY/\n    [...]\n\nand with OpenSSH, the tag object looks like:\n\n    $ git -C repo cat-file -p foo\n    object 64303f1200e1114d91f250cede3245364b07fce7\n    type commit\n    tag foo\n    tagger Taylor Blau <me@ttaylorr.com> 1651538975 -0400\n\n    foo\n    -----BEGIN SSH SIGNATURE-----\n    U1NIU0lHAAAAAQAAARcAAAAHc3NoLXJzYQAAAAMBAAEAAAEBAPKMdi3zhEm/soWtUUR9bZ\n    [...]\n\nnote the missing newline after \"-----BEGIN SSH SIGNATURE-----\", which\nconfuses ref-filter.c:find_subpos(). In particular, it seems to confuse\nthe code near the comment \"subject goes to first empty line before\nsignature begins\", since the OpenSSH-signed version of \"foo\" treats the\nentire message (including the signature itself) as the subject.\n\nThat puts buf after sigstart, meaning that *nonsiglen gets assigned to a\nnegative number and overflows. Luckily this just results in a bogus\nmalloc call which crashes us, so I don't think this is meaningfully\nexploitable.\n\nThe following diff seems to do the trick by restoring the intent of our\n\"scooch batch 'eol' if it goes beyond 'sigstart'\" to the case where we\njust assume the whole message is the subject. Folks more familiar with\nthis code are more than welcome to chime in ;).\n\n--- 8< ---\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 7838bd22b8..1d69e28d68 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1376,12 +1376,14 @@ static void find_subpos(const char *buf,\n \t*sub = buf;\n \t/* subject goes to first empty line before signature begins */\n \tif ((eol = strstr(*sub, \"\\n\\n\"))) {\n-\t\teol = eol < sigstart ? eol : sigstart;\n+\t\t;\n \t/* check if message uses CRLF */\n \t} else if (! (eol = strstr(*sub, \"\\r\\n\\r\\n\"))) {\n \t\t/* treat whole message as subject */\n \t\teol = strrchr(*sub, '\\0');\n \t}\n+\tif (sigstart < eol)\n+\t\teol = sigstart;\n \tbuf = eol;\n \t*sublen = buf - *sub;\n \t/* drop trailing newline, if present */\n--- >8 ---\n\nThanks,\nTaylor\n"}]}