From c39fc06b999305963600358f3f5e99698440cad2 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Mon, 10 Jan 2022 16:19:06 -0500 Subject: fmt-merge-msg: prevent use-after-free with signed tags 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. 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 Signed-off-by: Taylor Blau Signed-off-by: Junio C Hamano --- fmt-merge-msg.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fmt-merge-msg.c') diff --git a/fmt-merge-msg.c b/fmt-merge-msg.c index e4f7810be2..422bd0c055 100644 --- a/fmt-merge-msg.c +++ b/fmt-merge-msg.c @@ -541,7 +541,6 @@ static void fmt_merge_msg_sigs(struct strbuf *out) else strbuf_addstr(&sig, sigc.output); } - signature_check_clear(&sigc); if (!tag_number++) { fmt_tag_signature(&tagbuf, &sig, buf, len); @@ -565,6 +564,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out) } strbuf_release(&payload); strbuf_release(&sig); + signature_check_clear(&sigc); next: free(origbuf); } -- cgit v1.2.3