From: Taylor Blau <me@ttaylorr.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Brad Beam <brad.beam@b-rad.info>, git@vger.kernel.org
Subject: Re: Bug report - ssh signing causes git tag -l malloc failed
Date: Mon, 2 May 2022 21:02:53 -0400 [thread overview]
Message-ID: <YnB/Pam2ti1A8c0e@nand.local> (raw)
In-Reply-To: <xmqq7d73y97i.fsf@gitster.g>
On Mon, May 02, 2022 at 01:24:01PM -0700, Junio C Hamano wrote:
> Brad Beam <brad.beam@b-rad.info> writes:
>
> Thanks for a report.
>
> > What did you do before the bug happened? (Steps to reproduce your
> > issue)
> >
> > When using ssh signing `git tag -l --format='%(contents:body)'
> > <tag>` returns `fatal: Out of memory, malloc failed (tried to
> > allocate 18446744073709551323 bytes)`
>
> An obvious first follow-up question is if there is any difference in
> behaviour if another kind of signing (like PGP) is used.
It works when using gpg.format=openpgp, and breaks reliably when signing
using OpenSSH. The simplest reproduction I could come up with is:
git init repo
(
cd repo
>foo
git add foo
git commit -m "$(date)"
git tag -sam foo foo
)
I think the difference is that when signing with OpenPGP, the "foo" tag
looks like this:
$ git -C repo cat-file -p foo
object d9b42c39c8520b2b9d62d67aa4138e1a28ce7aee
type commit
tag foo
tagger Taylor Blau <me@ttaylorr.com> 1651538928 -0400
foo
-----BEGIN PGP SIGNATURE-----
iQEzBAABCgAdFiEEuSAhSeE5DcWB+EsnUHZwBbY/KHsFAmJwe/AACgkQUHZwBbY/
[...]
and with OpenSSH, the tag object looks like:
$ git -C repo cat-file -p foo
object 64303f1200e1114d91f250cede3245364b07fce7
type commit
tag foo
tagger Taylor Blau <me@ttaylorr.com> 1651538975 -0400
foo
-----BEGIN SSH SIGNATURE-----
U1NIU0lHAAAAAQAAARcAAAAHc3NoLXJzYQAAAAMBAAEAAAEBAPKMdi3zhEm/soWtUUR9bZ
[...]
note the missing newline after "-----BEGIN SSH SIGNATURE-----", which
confuses ref-filter.c:find_subpos(). In particular, it seems to confuse
the code near the comment "subject goes to first empty line before
signature begins", since the OpenSSH-signed version of "foo" treats the
entire message (including the signature itself) as the subject.
That puts buf after sigstart, meaning that *nonsiglen gets assigned to a
negative number and overflows. Luckily this just results in a bogus
malloc call which crashes us, so I don't think this is meaningfully
exploitable.
The following diff seems to do the trick by restoring the intent of our
"scooch batch 'eol' if it goes beyond 'sigstart'" to the case where we
just assume the whole message is the subject. Folks more familiar with
this code are more than welcome to chime in ;).
--- 8< ---
diff --git a/ref-filter.c b/ref-filter.c
index 7838bd22b8..1d69e28d68 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1376,12 +1376,14 @@ static void find_subpos(const char *buf,
*sub = buf;
/* subject goes to first empty line before signature begins */
if ((eol = strstr(*sub, "\n\n"))) {
- eol = eol < sigstart ? eol : sigstart;
+ ;
/* check if message uses CRLF */
} else if (! (eol = strstr(*sub, "\r\n\r\n"))) {
/* treat whole message as subject */
eol = strrchr(*sub, '\0');
}
+ if (sigstart < eol)
+ eol = sigstart;
buf = eol;
*sublen = buf - *sub;
/* drop trailing newline, if present */
--- >8 ---
Thanks,
Taylor
prev parent reply other threads:[~2022-05-03 1:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-02 19:51 Bug report - ssh signing causes git tag -l malloc failed Brad Beam
2022-05-02 20:24 ` Junio C Hamano
2022-05-03 1:02 ` Taylor Blau [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YnB/Pam2ti1A8c0e@nand.local \
--to=me@ttaylorr.com \
--cc=brad.beam@b-rad.info \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).