Taylor Blau <me@xxxxxxxxxxxx> writes:
> 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.
> 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@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
Will fast-track. Thanks.
