git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] fmt-merge-msg: prevent use-after-free with signed tags

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 10, 2022, 21:38 UTC
Message-ID
<xmqqsftvxodf.fsf@gitster.g>
In-Reply-To
<6e08b73d602853b3de71257117e85e32b96b5c19.1641849502.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 14 quoted lines
> When merging a signed tag, fmt_merge_msg_sigs() is responsible for
> populating the body of the merge message with the names of the signed
> tags, their signatures, and the validity of those signatures.
>
> In 02769437e1 (ssh signing: use sigc struct to pass payload,
> 2021-12-09), check_signature() was taught to pass the object payload via
> the sigc struct instead of passing the payload buffer separately.
>
> In effect, 02769437e1 causes buf, and sigc.payload to point at the same
> region in memory. This causes a problem for fmt_tag_signature(), which
> wants to read from this location, since it is freed beforehand by
> signature_check_clear() (which frees it via sigc's `payload` member).
>
> That makes the subsequent use in fmt_tag_signature() a use-after-free.
Very clearly described.
Show 22 quoted lines
> As a result, merge messages did not contain the body of any signed tags.
> Luckily, they tend not to contain garbage, either, since the result of
> strstr()-ing the object buffer in fmt_tag_signature() is guarded:
>
>     const char *tag_body = strstr(buf, "\n\n");
>     if (tag_body) {
>       tag_body += 2;
>       strbuf_add(tagbuf, tag_body, buf + len - tag_body);
>     }
>
> Unfortunately, the tests in t6200 did not catch this at the time because
> they do not search for the body of signed tags in fmt-merge-msg's
> output.
>
> Resolve this by waiting to call signature_check_clear() until after its
> contents can be safely discarded. Harden ourselves against any future
> regressions in this area by making sure we can find signed tag messages
> in the output of fmt-merge-msg, too.
>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
Will fast-track.  Thanks.
Previous: Taylor BlauNext: Fabian Stelzer
Message 6 of 8 in “git ssh signing changed broke tag merge message contents”
  1. Linus TorvaldsJan 10, 2022
  2. Taylor BlauJan 10, 2022
  3. Linus TorvaldsJan 10, 2022
  4. Junio C HamanoJan 10, 2022
  5. fmt-merge-msg: prevent use-after-free with signed tagsTaylor Blau, Jan 10, 2022
  6. Junio C HamanoJan 10, 2022
  7. Fabian StelzerJan 11, 2022
  8. Taylor BlauJan 11, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.