send-pack: fix leaking push cert nonce

When retrieving the push cert nonce from the server, we first store the constant returned by `server_feature_value()` and then, if the nonce is valid, we duplicate the nonce memory to a NUL-terminated string, so that we can pass it to `generate_push_cert()`. We never free the latter and thus cause a memory leak. Fix this by storing the limited-lifetime nonce into a scope-local variable such that the long-lived, allocated nonce can be easily freed without having to cast away its constness. This leak was exposed by t5534, but fixing it is not sufficient to make the whole test suite leak free. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Sep 5, 2024 at 12:09 UTC 49d47eb5416d22f185877a57380a1ffc28f172e1
1 file changed +7 -5
send-pack.c
+7 -5
@@ -501,7 +501,7 @@ int send_pack(struct send_pack_args *args,
501 unsigned cmds_sent = 0;
502 int ret;
503 struct async demux;
504 - const char *push_cert_nonce = NULL;
504 + char *push_cert_nonce = NULL;
505 struct packet_reader reader;
506 int use_bitmaps;
507
@@ -550,10 +550,11 @@ int send_pack(struct send_pack_args *args,
550
551 if (args->push_cert != SEND_PACK_PUSH_CERT_NEVER) {
552 size_t len;
553 - push_cert_nonce = server_feature_value("push-cert", &len);
554 - if (push_cert_nonce) {
555 - reject_invalid_nonce(push_cert_nonce, len);
556 - push_cert_nonce = xmemdupz(push_cert_nonce, len);
553 + const char *nonce = server_feature_value("push-cert", &len);
554 +
555 + if (nonce) {
556 + reject_invalid_nonce(nonce, len);
557 + push_cert_nonce = xmemdupz(nonce, len);
558 } else if (args->push_cert == SEND_PACK_PUSH_CERT_ALWAYS) {
559 die(_("the receiving end does not support --signed push"));
560 } else if (args->push_cert == SEND_PACK_PUSH_CERT_IF_ASKED) {
@@ -771,5 +772,6 @@ out:
772 oid_array_clear(&commons);
773 strbuf_release(&req_buf);
774 strbuf_release(&cap_buf);
775 + free(push_cert_nonce);
776 return ret;
777 }