From ac45d995f3329e5a7e85bf103bd94b48107b3803 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 1 Oct 2023 21:40:21 -0500 Subject: [PATCH] object-file-convert: don't leak when converting tag objects Upon close examination I discovered that while brian's code to convert tag objects was functionally correct, it leaked memory. Rearrange the code so that all error checking happens before any memory is allocated. Add code to release the temporary strbufs the code uses. The code pretty much assumes the tag object ends with a newline, so add an explict test to verify that is the case. Signed-off-by: Eric W. Biederman Signed-off-by: Junio C Hamano --- object-file-convert.c | 59 +++++++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/object-file-convert.c b/object-file-convert.c index 089b68442d..79e8e211ff 100644 --- a/object-file-convert.c +++ b/object-file-convert.c @@ -90,44 +90,49 @@ static int convert_tag_object(struct strbuf *out, const struct git_hash_algo *to, const char *buffer, size_t size) { - struct strbuf payload = STRBUF_INIT, temp = STRBUF_INIT, oursig = STRBUF_INIT, othersig = STRBUF_INIT; + struct strbuf payload = STRBUF_INIT, oursig = STRBUF_INIT, othersig = STRBUF_INIT; + const int entry_len = from->hexsz + 7; size_t payload_size; struct object_id oid, mapped_oid; const char *p; - /* Add some slop for longer signature header in the new algorithm. */ - strbuf_grow(out, size + 7); - - /* Is there a signature for our algorithm? */ - payload_size = parse_signed_buffer(buffer, size); - strbuf_add(&payload, buffer, payload_size); - if (payload_size != size) { - /* Yes, there is. */ - strbuf_add(&oursig, buffer + payload_size, size - payload_size); - } - /* Now, is there a signature for the other algorithm? */ - if (parse_buffer_signed_by_header(payload.buf, payload.len, &temp, &othersig, to)) { - /* Yes, there is. */ - strbuf_swap(&payload, &temp); - strbuf_release(&temp); - } - - /* - * Our payload is now in payload and we may have up to two signatrures - * in oursig and othersig. - */ - if (strncmp(payload.buf, "object ", 7) || payload.buf[from->hexsz + 7] != '\n') + /* Consume the object line */ + if ((entry_len >= size) || + memcmp(buffer, "object ", 7) || buffer[entry_len] != '\n') return error("bogus tag object"); - if (parse_oid_hex_algop(payload.buf + 7, &oid, &p, from) < 0) + if (parse_oid_hex_algop(buffer + 7, &oid, &p, from) < 0) return error("bad tag object ID"); if (repo_oid_to_algop(the_repository, &oid, to, &mapped_oid)) return error("unable to map tree %s in tag object", oid_to_hex(&oid)); - strbuf_addf(out, "object %s", oid_to_hex(&mapped_oid)); - strbuf_add(out, p, payload.len - (p - payload.buf)); - strbuf_addbuf(out, &othersig); + size -= ((p + 1) - buffer); + buffer = p + 1; + + /* Is there a signature for our algorithm? */ + payload_size = parse_signed_buffer(buffer, size); + if (payload_size != size) { + /* Yes, there is. */ + strbuf_add(&oursig, buffer + payload_size, size - payload_size); + } + + /* Now, is there a signature for the other algorithm? */ + parse_buffer_signed_by_header(buffer, payload_size, &payload, &othersig, to); + /* + * Our payload is now in payload and we may have up to two signatrures + * in oursig and othersig. + */ + + /* Add some slop for longer signature header in the new algorithm. */ + strbuf_grow(out, (7 + to->hexsz + 1) + size + 7); + strbuf_addf(out, "object %s\n", oid_to_hex(&mapped_oid)); + strbuf_addbuf(out, &payload); if (oursig.len) add_header_signature(out, &oursig, from); + strbuf_addbuf(out, &othersig); + + strbuf_release(&payload); + strbuf_release(&othersig); + strbuf_release(&oursig); return 0; }