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>
Patrick Steinhardt committed
Sep 5, 2024 at 12:09 UTC
b8849e236f7a32d43ab3ba087587a336d69329b0
6 files changed
+30
-19
builtin/tag.c
+2
-1
@@ -160,7 +160,7 @@ static int do_sign(struct strbuf *buffer, struct object_id **compat_oid,
160
const struct git_hash_algo *compat = the_repository->compat_hash_algo;
161
struct strbuf sig = STRBUF_INIT, compat_sig = STRBUF_INIT;
162
struct strbuf compat_buf = STRBUF_INIT;
163
- const char *keyid = get_signing_key();
163
+ char *keyid = get_signing_key();
164
int ret = -1;
165
166
if (sign_buffer(buffer, &sig, keyid))
@@ -190,6 +190,7 @@ out:
190
strbuf_release(&sig);
191
strbuf_release(&compat_sig);
192
strbuf_release(&compat_buf);
193
+ free(keyid);
194
return ret;
195
}
196
commit.c
+6
-3
@@ -1150,11 +1150,14 @@ int add_header_signature(struct strbuf *buf, struct strbuf *sig, const struct gi
1150
1151
static int sign_commit_to_strbuf(struct strbuf *sig, struct strbuf *buf, const char *keyid)
1152
{
1153
+ char *keyid_to_free = NULL;
1154
+ int ret = 0;
1155
if (!keyid || !*keyid)
1154
- keyid = get_signing_key();
1156
+ keyid = keyid_to_free = get_signing_key();
1157
if (sign_buffer(buf, sig, keyid))
1156
- return -1;
1157
- return 0;
1158
+ ret = -1;
1159
+ free(keyid_to_free);
1160
+ return ret;
1161
}
1162
1163
int parse_signed_commit(const struct commit *commit,
gpg-interface.c
+15
-11
@@ -45,8 +45,8 @@ struct gpg_format {
45
size_t signature_size);
46
int (*sign_buffer)(struct strbuf *buffer, struct strbuf *signature,
47
const char *signing_key);
48
- const char *(*get_default_key)(void);
49
- const char *(*get_key_id)(void);
48
+ char *(*get_default_key)(void);
49
+ char *(*get_key_id)(void);
50
};
51
52
static const char *openpgp_verify_args[] = {
@@ -86,9 +86,9 @@ static int sign_buffer_gpg(struct strbuf *buffer, struct strbuf *signature,
86
static int sign_buffer_ssh(struct strbuf *buffer, struct strbuf *signature,
87
const char *signing_key);
88
89
-static const char *get_default_ssh_signing_key(void);
89
+static char *get_default_ssh_signing_key(void);
90
91
-static const char *get_ssh_key_id(void);
91
+static char *get_ssh_key_id(void);
92
93
static struct gpg_format gpg_format[] = {
94
{
@@ -847,7 +847,7 @@ static char *get_ssh_key_fingerprint(const char *signing_key)
847
}
848
849
/* Returns the first public key from an ssh-agent to use for signing */
850
-static const char *get_default_ssh_signing_key(void)
850
+static char *get_default_ssh_signing_key(void)
851
{
852
struct child_process ssh_default_key = CHILD_PROCESS_INIT;
853
int ret = -1;
@@ -899,12 +899,16 @@ static const char *get_default_ssh_signing_key(void)
899
return default_key;
900
}
901
902
-static const char *get_ssh_key_id(void) {
903
- return get_ssh_key_fingerprint(get_signing_key());
902
+static char *get_ssh_key_id(void)
903
+{
904
+ char *signing_key = get_signing_key();
905
+ char *key_id = get_ssh_key_fingerprint(signing_key);
906
+ free(signing_key);
907
+ return key_id;
908
}
909
910
/* Returns a textual but unique representation of the signing key */
907
-const char *get_signing_key_id(void)
911
+char *get_signing_key_id(void)
912
{
913
gpg_interface_lazy_init();
914
@@ -916,17 +920,17 @@ const char *get_signing_key_id(void)
920
return get_signing_key();
921
}
922
919
-const char *get_signing_key(void)
923
+char *get_signing_key(void)
924
{
925
gpg_interface_lazy_init();
926
927
if (configured_signing_key)
924
- return configured_signing_key;
928
+ return xstrdup(configured_signing_key);
929
if (use_format->get_default_key) {
930
return use_format->get_default_key();
931
}
932
929
- return git_committer_info(IDENT_STRICT | IDENT_NO_DATE);
933
+ return xstrdup(git_committer_info(IDENT_STRICT | IDENT_NO_DATE));
934
}
935
936
const char *gpg_trust_level_to_str(enum signature_trust_level level)
gpg-interface.h
+2
-2
@@ -80,13 +80,13 @@ int sign_buffer(struct strbuf *buffer, struct strbuf *signature,
80
const char *gpg_trust_level_to_str(enum signature_trust_level level);
81
82
void set_signing_key(const char *);
83
-const char *get_signing_key(void);
83
+char *get_signing_key(void);
84
85
/*
86
* Returns a textual unique representation of the signing key in use
87
* Either a GPG KeyID or a SSH Key Fingerprint
88
*/
89
-const char *get_signing_key_id(void);
89
+char *get_signing_key_id(void);
90
int check_signature(struct signature_check *sigc,
91
const char *signature, size_t slen);
92
void print_signature_buffer(const struct signature_check *sigc,
send-pack.c
+4
-2
@@ -348,7 +348,8 @@ static int generate_push_cert(struct strbuf *req_buf,
348
{
349
const struct ref *ref;
350
struct string_list_item *item;
351
- char *signing_key_id = xstrdup(get_signing_key_id());
351
+ char *signing_key_id = get_signing_key_id();
352
+ char *signing_key = get_signing_key();
353
const char *cp, *np;
354
struct strbuf cert = STRBUF_INIT;
355
int update_seen = 0;
@@ -381,7 +382,7 @@ static int generate_push_cert(struct strbuf *req_buf,
382
if (!update_seen)
383
goto free_return;
384
384
- if (sign_buffer(&cert, &cert, get_signing_key()))
385
+ if (sign_buffer(&cert, &cert, signing_key))
386
die(_("failed to sign the push certificate"));
387
388
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,
395
396
free_return:
397
free(signing_key_id);
398
+ free(signing_key);
399
strbuf_release(&cert);
400
return update_seen;
401
}
t/t5534-push-signed.sh
+1
@@ -5,6 +5,7 @@ test_description='signed push'
5
GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
6
export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
7
8
+TEST_PASSES_SANITIZE_LEAK=true
9
. ./test-lib.sh
10
. "$TEST_DIRECTORY"/lib-gpg.sh
11