gpg-interface: fix misdesigned signing key interfaces

The interfaces to retrieve signing keys and their IDs are misdesigned as
they return string constants even though they indeed allocate memory,
which leads to memory leaks. Refactor the code to instead always return
allocated strings and let the callers free them accordingly.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/tag.c b/builtin/tag.c
index a1fb218512cc1a072682a33411a6da29313e2446..ab3b500543d769064633deec6715ed71e6125476 100644 (file)
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -160,7 +160,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
        const struct git_hash_algo *compat = the_repository->compat_hash_algo;
        struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT;
        struct strbuf compat_buf = STRBUF_INIT;
-       const char *keyid = get_signing_key();
+       char *keyid = get_signing_key();
        int ret = -1;
 
        if (sign_buffer(buffer, &sig, keyid))
@@ -190,6 +190,7 @@ out:
        strbuf_release(&sig);
        strbuf_release(&compat_sig);
        strbuf_release(&compat_buf);
+       free(keyid);
        return ret;
 }
 

diff --git a/commit.c b/commit.c
index 24ab5c1b50939c31a9f586f40d475252ebac4565..ec9efc189d53421241a4385ffb0d0278ebdecfee 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -1150,11 +1150,14 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
 
 static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
 {
+       char *keyid_to_free = NULL;
+       int ret = 0;
        if (!keyid || !*keyid)
-               keyid = get_signing_key();
+               keyid = keyid_to_free = get_signing_key();
        if (sign_buffer(buf, sig, keyid))
-               return -1;
-       return 0;
+               ret = -1;
+       free(keyid_to_free);
+       return ret;
 }
 
 int parse_signed_commit(const struct commit *commit,

diff --git a/gpg-interface.c b/gpg-interface.c
index 6587085cd19fc0cd7994262a97f75661945a303b..cf6126b5aa08e8cd3d22dda6eef0edeffe553944 100644 (file)
--- a/gpg-interface.c
+++ b/gpg-interface.c
@@ -45,8 +45,8 @@ struct gpg_format {
                                    size_t signature_size);
        int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,
                           const char *signing_key);
-       const char *(*get_default_key)(void);
-       const char *(*get_key_id)(void);
+       char *(*get_default_key)(void);
+       char *(*get_key_id)(void);
 };
 
 static const char *openpgp_verify_args[] = {
@@ -86,9 +86,9 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
 static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
                           const char *signing_key);
 
-static const char *get_default_ssh_signing_key(void);
+static char *get_default_ssh_signing_key(void);
 
-static const char *get_ssh_key_id(void);
+static char *get_ssh_key_id(void);
 
 static struct gpg_format gpg_format[] = {
        {
@@ -847,7 +847,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
 }
 
 /* Returns the first public key from an ssh-agent to use for signing */
-static const char *get_default_ssh_signing_key(void)
+static char *get_default_ssh_signing_key(void)
 {
        struct child_process ssh_default_key = CHILD_PROCESS_INIT;
        int ret = -1;
@@ -899,12 +899,16 @@ static const char *get_default_ssh_signing_key(void)
        return default_key;
 }
 
-static const char *get_ssh_key_id(void) {
-       return get_ssh_key_fingerprint(get_signing_key());
+static char *get_ssh_key_id(void)
+{
+       char *signing_key = get_signing_key();
+       char *key_id = get_ssh_key_fingerprint(signing_key);
+       free(signing_key);
+       return key_id;
 }
 
 /* Returns a textual but unique representation of the signing key */
-const char *get_signing_key_id(void)
+char *get_signing_key_id(void)
 {
        gpg_interface_lazy_init();
 
@@ -916,17 +920,17 @@ const char *get_signing_key_id(void)
        return get_signing_key();
 }
 
-const char *get_signing_key(void)
+char *get_signing_key(void)
 {
        gpg_interface_lazy_init();
 
        if (configured_signing_key)
-               return configured_signing_key;
+               return xstrdup(configured_signing_key);
        if (use_format->get_default_key) {
                return use_format->get_default_key();
        }
 
-       return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
+       return xstrdup(git_committer_info(IDENT_STRICT | IDENT_NO_DATE));
 }
 
 const char *gpg_trust_level_to_str(enum signature_trust_level level)

diff --git a/gpg-interface.h b/gpg-interface.h
index 7cd98161f7f21b5946daa6834617d4876745119f..e09f12e8d04d925faa8613d340c50fea71ca0239 100644 (file)
--- a/gpg-interface.h
+++ b/gpg-interface.h
@@ -80,13 +80,13 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
 const char *gpg_trust_level_to_str(enum signature_trust_level level);
 
 void set_signing_key(const char *);
-const char *get_signing_key(void);
+char *get_signing_key(void);
 
 /*
  * Returns a textual unique representation of the signing key in use
  * Either a GPG KeyID or a SSH Key Fingerprint
  */
-const char *get_signing_key_id(void);
+char *get_signing_key_id(void);
 int check_signature(struct signature_check *sigc,
                    const char *signature, size_t slen);
 void print_signature_buffer(const struct signature_check *sigc,

diff --git a/send-pack.c b/send-pack.c
index c37f6ab3c071ca4ffe27c769834bf25485851636..31a62e6a98c59788d190cab1a89d408727aa9de0 100644 (file)
--- a/send-pack.c
+++ b/send-pack.c
@@ -348,7 +348,8 @@ static int generate_push_cert(struct strbuf *req_buf,
 {
        const struct ref *ref;
        struct string_list_item *item;
-       char *signing_key_id = xstrdup(get_signing_key_id());
+       char *signing_key_id = get_signing_key_id();
+       char *signing_key = get_signing_key();
        const char *cp, *np;
        struct strbuf cert = STRBUF_INIT;
        int update_seen = 0;
@@ -381,7 +382,7 @@ static int generate_push_cert(struct strbuf *req_buf,
        if (!update_seen)
                goto free_return;
 
-       if (sign_buffer(&cert, &cert, get_signing_key()))
+       if (sign_buffer(&cert, &cert, signing_key))
                die(_("failed to sign the push certificate"));
 
        packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string);
@@ -394,6 +395,7 @@ static int generate_push_cert(struct strbuf *req_buf,
 
 free_return:
        free(signing_key_id);
+       free(signing_key);
        strbuf_release(&cert);
        return update_seen;
 }

diff --git a/t/t5534-push-signed.sh b/t/t5534-push-signed.sh
index c91a62b77afcfba1bf1228c33717db77c7e45318..d43aee0c327c0ba4680db01a06070b5688a1dd72 100755 (executable)
--- a/t/t5534-push-signed.sh
+++ b/t/t5534-push-signed.sh
@@ -5,6 +5,7 @@ test_description='signed push'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-gpg.sh