diff -up gnupg-2.2.21/common/server-help.c.coverity gnupg-2.2.21/common/server-help.c --- gnupg-2.2.21/common/server-help.c.coverity 2019-02-11 10:59:34.000000000 +0100 +++ gnupg-2.2.21/common/server-help.c 2020-07-20 17:09:57.416148768 +0200 @@ -156,7 +156,7 @@ get_option_value (char *line, const char *pend = 0; *r_value = xtrystrdup (p); *pend = c; - if (!p) + if (!*r_value) return my_error_from_syserror (); return 0; } From 912e77f07d8a42d7ad001eb3df76f6932ccfa857 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 7 Apr 2021 17:37:51 +0200 Subject: [PATCH GnuPG 01/19] agent: Avoid memory leaks * agent/command.c (cmd_genkey): use goto leave instead of return * agent/cvt-openpgp.c (convert_from_openpgp_main): use goto leave instead of return * agent/genkey.c (agent_ask_new_passphrase): fix typo to free correct pointer (agent_genkey): release memory * agent/gpg-agent.c (check_own_socket): free sockname * agent/protect-tool.c (read_key): free buf (agent_askpin): free passphrase * agent/protect.c (merge_lists): free newlist -- Signed-off-by: Jakub Jelen --- agent/command.c | 2 +- agent/cvt-openpgp.c | 5 ++++- agent/genkey.c | 7 +++++-- agent/gpg-agent.c | 10 ++++++++-- agent/protect-tool.c | 6 +++++- agent/protect.c | 5 ++++- 6 files changed, 27 insertions(+), 8 deletions(-) diff --git a/agent/command.c b/agent/command.c index 93cd281e7..b9a1ed038 100644 --- a/agent/command.c +++ b/agent/command.c @@ -1021,7 +1021,7 @@ cmd_genkey (assuan_context_t ctx, char *line) if (!rc) rc = assuan_inquire (ctx, "KEYPARAM", &value, &valuelen, MAXLEN_KEYPARAM); if (rc) - return rc; + goto leave; init_membuf (&outbuf, 512); diff --git a/agent/cvt-openpgp.c b/agent/cvt-openpgp.c index 3da553f95..53c88154b 100644 --- a/agent/cvt-openpgp.c +++ b/agent/cvt-openpgp.c @@ -964,7 +964,10 @@ convert_from_openpgp_main (ctrl_t ctrl, gcry_sexp_t s_pgp, int dontcare_exist, pi = xtrycalloc_secure (1, sizeof (*pi) + MAX_PASSPHRASE_LEN + 1); if (!pi) - return gpg_error_from_syserror (); + { + err = gpg_error_from_syserror (); + goto leave; + } pi->max_length = MAX_PASSPHRASE_LEN + 1; pi->min_digits = 0; /* We want a real passphrase. */ pi->max_digits = 16; diff --git a/agent/genkey.c b/agent/genkey.c index 9b47f0fac..c7cfc6910 100644 --- a/agent/genkey.c +++ b/agent/genkey.c @@ -363,7 +363,7 @@ agent_ask_new_passphrase (ctrl_t ctrl, const char *prompt, if (!pi2) { err = gpg_error_from_syserror (); - xfree (pi2); + xfree (pi); return err; } pi->max_length = MAX_PASSPHRASE_LEN + 1; @@ -465,7 +465,10 @@ agent_genkey (ctrl_t ctrl, const char *cache_nonce, time_t timestamp, "protect your new key"), &passphrase_buffer); if (rc) - return rc; + { + gcry_sexp_release (s_keyparam); + return rc; + } passphrase = passphrase_buffer; } diff --git a/agent/gpg-agent.c b/agent/gpg-agent.c index 1285db995..8f504191b 100644 --- a/agent/gpg-agent.c +++ b/agent/gpg-agent.c @@ -3214,11 +3214,17 @@ check_own_socket (void) sockname = make_filename_try (gnupg_socketdir (), GPG_AGENT_SOCK_NAME, NULL); if (!sockname) - return; /* Out of memory. */ + { + xfree (sockname); + return; /* Out of memory. */ + } err = npth_attr_init (&tattr); if (err) - return; + { + xfree (sockname); + return; + } npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED); err = npth_create (&thread, &tattr, check_own_socket_thread, sockname); if (err) diff --git a/agent/protect-tool.c b/agent/protect-tool.c index 1fcbd119f..bb17033a8 100644 --- a/agent/protect-tool.c +++ b/agent/protect-tool.c @@ -319,6 +319,7 @@ read_key (const char *fname) if (buflen >= 4 && !memcmp (buf, "Key:", 4)) { log_error ("Extended key format is not supported by this tool\n"); + xfree (buf); return NULL; } key = make_canonical (fname, buf, buflen); @@ -793,7 +794,10 @@ agent_askpin (ctrl_t ctrl, passphrase = get_passphrase (0); size = strlen (passphrase); if (size >= pininfo->max_length) - return gpg_error (GPG_ERR_TOO_LARGE); + { + xfree (passphrase); + return gpg_error (GPG_ERR_TOO_LARGE); + } memcpy (&pininfo->pin, passphrase, size); xfree (passphrase); diff --git a/agent/protect.c b/agent/protect.c index 76ead444b..50b10eb26 100644 --- a/agent/protect.c +++ b/agent/protect.c @@ -949,7 +949,10 @@ merge_lists (const unsigned char *protectedkey, /* Copy the cleartext. */ s = cleartext; if (*s != '(' && s[1] != '(') - return gpg_error (GPG_ERR_BUG); /*we already checked this */ + { + xfree (newlist); + return gpg_error (GPG_ERR_BUG); /*we already checked this */ + } s += 2; startpos = s; while ( *s == '(' ) -- 2.30.2 From 93dc0474ea35c0f8f93e0c5eee14cf0157b0d896 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 7 Apr 2021 18:54:02 +0200 Subject: [PATCH GnuPG 02/19] dirmgr: clean up memory on error code paths * dirmgr/crlcache.c (finish_sig_check): goto leave instead of return * dirmgr/http.c (send_request): free authstr and proxy_authstr * dirmgr/ldap.c (start_cert_fetch_ldap): free proxy * dirmgr/ocsp.c (check_signature): release s_hash -- Signed-off-by: Jakub Jelen --- dirmngr/crlcache.c | 9 ++++++--- dirmngr/http.c | 6 +++++- dirmngr/ldap.c | 6 ++++-- dirmngr/ocsp.c | 1 + 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/dirmngr/crlcache.c b/dirmngr/crlcache.c index 9d18b721f..d508e173f 100644 --- a/dirmngr/crlcache.c +++ b/dirmngr/crlcache.c @@ -1725,7 +1725,8 @@ finish_sig_check (ksba_crl_t crl, gcry_md_hd_t md, int algo, { log_error ("hash algo mismatch: %d announced but %d used\n", algo, hashalgo); - return gpg_error (GPG_ERR_INV_CRL); + err = gpg_error (GPG_ERR_INV_CRL); + goto leave; } /* Add some restrictions; see ../sm/certcheck.c for details. */ switch (algo) @@ -1741,14 +1742,16 @@ finish_sig_check (ksba_crl_t crl, gcry_md_hd_t md, int algo, default: log_error ("PSS hash algorithm '%s' rejected\n", gcry_md_algo_name (algo)); - return gpg_error (GPG_ERR_DIGEST_ALGO); + err = gpg_error (GPG_ERR_DIGEST_ALGO); + goto leave; } if (gcry_md_get_algo_dlen (algo) != saltlen) { log_error ("PSS hash algorithm '%s' rejected due to salt length %u\n", gcry_md_algo_name (algo), saltlen); - return gpg_error (GPG_ERR_DIGEST_ALGO); + err = gpg_error (GPG_ERR_DIGEST_ALGO); + goto leave; } } diff --git a/dirmngr/http.c b/dirmngr/http.c index f7f65303b..74ce5f465 100644 --- a/dirmngr/http.c +++ b/dirmngr/http.c @@ -2208,7 +2208,11 @@ send_request (ctrl_t ctrl, http_t hd, const char *httphost, const char *auth, p = build_rel_path (hd->uri); if (!p) - return gpg_err_make (default_errsource, gpg_err_code_from_syserror ()); + { + xfree (authstr); + xfree (proxy_authstr); + return gpg_err_make (default_errsource, gpg_err_code_from_syserror ()); + } if (http_proxy && *http_proxy) { diff --git a/dirmngr/ldap.c b/dirmngr/ldap.c index ffe54bade..96abc89d0 100644 --- a/dirmngr/ldap.c +++ b/dirmngr/ldap.c @@ -563,8 +563,10 @@ start_cert_fetch_ldap (ctrl_t ctrl, cert_fetch_context_t *r_context, use_ldaps = server->use_ldaps; } else /* Use a default server. */ - return gpg_error (GPG_ERR_NOT_IMPLEMENTED); - + { + xfree (proxy); + return gpg_error (GPG_ERR_NOT_IMPLEMENTED); + } if (!base) base = ""; diff --git a/dirmngr/ocsp.c b/dirmngr/ocsp.c index 6ed180955..6864f9854 100644 --- a/dirmngr/ocsp.c +++ b/dirmngr/ocsp.c @@ -534,6 +534,7 @@ check_signature (ctrl_t ctrl, err = ksba_ocsp_get_responder_id (ocsp, &name, &keyid); if (err) { + gcry_sexp_release (s_hash); log_error (_("error getting responder ID: %s\n"), gcry_strerror (err)); return err; -- 2.30.2 From 7a707a3eff1c3fbe17a74337776871f408377cee Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Fri, 9 Apr 2021 16:13:07 +0200 Subject: [PATCH GnuPG 03/19] g10: Fix memory leaks * g10/card-util.c (change_pin): free answer on errors (ask_card_keyattr): free answer on error * g10/cpr.c (do_get_from_fd): free string * g10/gpg.c (check_permissions): free dir on weird error * g10/import.c (append_new_uid): release knode * g10/keyedit.c (menu_set_keyserver_url): free answer (menu_set_keyserver_url): free user * g10/keygen.c (print_status_key_not_created): move allocation after sanity check (ask_expire_interval): free answer (card_store_key_with_backup): goto leave instaed of return * g10/keyserver.c (parse_keyserver_uri): goto fail instead of return * g10/revoke.c (gen_desig_revoke): release kdbhd (gen_desig_revoke): free answer * g10/tofu.c (ask_about_binding): free sqerr and response * g10/trustdb.c (ask_ownertrust): free pk -- Signed-off-by: Jakub Jelen --- g10/card-util.c | 14 +++++++++++--- g10/cpr.c | 6 +++++- g10/gpg.c | 1 + g10/import.c | 5 ++++- g10/keyedit.c | 8 +++++++- g10/keygen.c | 15 +++++++++++---- g10/keyserver.c | 2 +- g10/revoke.c | 6 +++++- g10/tofu.c | 4 ++++ g10/trustdb.c | 1 + 10 files changed, 50 insertions(+), 12 deletions(-) diff --git a/g10/card-util.c b/g10/card-util.c index 36f096f06..c7df8380d 100644 --- a/g10/card-util.c +++ b/g10/card-util.c @@ -127,7 +127,7 @@ change_pin (int unblock_v2, int allow_admin) else for (;;) { - char *answer; + char *answer = NULL; tty_printf ("\n"); tty_printf ("1 - change PIN\n" @@ -140,7 +140,10 @@ change_pin (int unblock_v2, int allow_admin) answer = cpr_get("cardutil.change_pin.menu",_("Your selection? ")); cpr_kill_prompt(); if (strlen (answer) != 1) - continue; + { + xfree (answer); + continue; + } if (*answer == '1') { @@ -185,8 +188,10 @@ change_pin (int unblock_v2, int allow_admin) } else if (*answer == 'q' || *answer == 'Q') { + xfree (answer); break; } + xfree (answer); } agent_release_card_info (&info); @@ -1450,7 +1455,10 @@ ask_card_keyattr (int keyno, const struct key_attr *current) algo = *answer? atoi (answer) : 0; if (!*answer || algo == 1 || algo == 2) - break; + { + xfree (answer); + break; + } else tty_printf (_("Invalid selection.\n")); } diff --git a/g10/cpr.c b/g10/cpr.c index 5a39913c5..002656b82 100644 --- a/g10/cpr.c +++ b/g10/cpr.c @@ -527,7 +527,11 @@ do_get_from_fd ( const char *keyword, int hidden, int getbool ) write_status (STATUS_GOT_IT); if (getbool) /* Fixme: is this correct??? */ - return (string[0] == 'Y' || string[0] == 'y') ? "" : NULL; + { + char *rv = (string[0] == 'Y' || string[0] == 'y') ? "" : NULL; + xfree (string); + return rv; + } return string; } diff --git a/g10/gpg.c b/g10/gpg.c index f5623be76..186845cea 100644 --- a/g10/gpg.c +++ b/g10/gpg.c @@ -1601,6 +1601,7 @@ check_permissions (const char *path, int item) if (gnupg_stat (dir,&dirbuf) || !S_ISDIR (dirbuf.st_mode)) { /* Weird error */ + xfree(dir); ret=1; goto end; } diff --git a/g10/import.c b/g10/import.c index 821ddf0d4..951c33d81 100644 --- a/g10/import.c +++ b/g10/import.c @@ -4524,7 +4524,10 @@ append_new_uid (unsigned int options, err = insert_key_origin_uid (n->pkt->pkt.user_id, curtime, origin, url); if (err) - return err; + { + release_kbnode (n); + return err; + } } if (n_where) diff --git a/g10/keyedit.c b/g10/keyedit.c index 531d3e128..902741b5f 100644 --- a/g10/keyedit.c +++ b/g10/keyedit.c @@ -5307,7 +5307,10 @@ menu_set_keyserver_url (ctrl_t ctrl, const char *url, kbnode_t pub_keyblock) } if (ascii_strcasecmp (answer, "none") == 0) - uri = NULL; + { + xfree (answer); + uri = NULL; + } else { struct keyserver_spec *keyserver = NULL; @@ -5379,12 +5382,14 @@ menu_set_keyserver_url (ctrl_t ctrl, const char *url, kbnode_t pub_keyblock) uri ? _("Are you sure you want to replace it? (y/N) ") : _("Are you sure you want to delete it? (y/N) "))) + xfree (user); continue; } else if (uri == NULL) { /* There is no current keyserver URL, so there is no point in trying to un-set it. */ + xfree (user); continue; } @@ -5397,6 +5402,7 @@ menu_set_keyserver_url (ctrl_t ctrl, const char *url, kbnode_t pub_keyblock) log_error ("update_keysig_packet failed: %s\n", gpg_strerror (rc)); xfree (uri); + xfree (user); return 0; } /* replace the packet */ diff --git a/g10/keygen.c b/g10/keygen.c index 5d85c05d4..f1e4d3638 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -237,12 +237,13 @@ print_status_key_not_created (const char *handle) static gpg_error_t write_uid (kbnode_t root, const char *s) { - PACKET *pkt = xmalloc_clear (sizeof *pkt); + PACKET *pkt = NULL; size_t n = strlen (s); if (n > MAX_UID_PACKET_LENGTH - 10) return gpg_error (GPG_ERR_INV_USER_ID); + pkt = xmalloc_clear (sizeof *pkt); pkt->pkttype = PKT_USER_ID; pkt->pkt.user_id = xmalloc_clear (sizeof *pkt->pkt.user_id + n); pkt->pkt.user_id->len = n; @@ -2860,7 +2861,10 @@ ask_expire_interval(int object,const char *def_expire) xfree(prompt); if(*answer=='\0') - answer=xstrdup(def_expire); + { + xfree (answer); + answer = xstrdup (def_expire); + } } cpr_kill_prompt(); trim_spaces(answer); @@ -5238,12 +5242,15 @@ card_store_key_with_backup (ctrl_t ctrl, PKT_public_key *sub_psk, epoch2isotime (timestamp, (time_t)sk->timestamp); err = hexkeygrip_from_pk (sk, &hexgrip); if (err) - return err; + goto leave; memset(&info, 0, sizeof (info)); rc = agent_scd_getattr ("SERIALNO", &info); if (rc) - return (gpg_error_t)rc; + { + err = (gpg_error_t)rc; + goto leave; + } rc = agent_keytocard (hexgrip, 2, 1, info.serialno, timestamp); xfree (info.serialno); diff --git a/g10/keyserver.c b/g10/keyserver.c index c56021691..a20ebf24e 100644 --- a/g10/keyserver.c +++ b/g10/keyserver.c @@ -284,7 +284,7 @@ parse_keyserver_uri (const char *string,int require_scheme) if(*idx=='\0' || *idx=='[') { if(require_scheme) - return NULL; + goto fail; /* Assume HKP if there is no scheme */ assume_hkp=1; diff --git a/g10/revoke.c b/g10/revoke.c index c0a003b6f..d6cbf93cb 100644 --- a/g10/revoke.c +++ b/g10/revoke.c @@ -435,6 +435,7 @@ gen_desig_revoke (ctrl_t ctrl, const char *uname, strlist_t locusr) iobuf_close(out); release_revocation_reason_info( reason ); release_armor_context (afx); + keydb_release (kdbhd); return rc; } @@ -804,7 +805,10 @@ ask_revocation_reason( int key_rev, int cert_rev, int hint ) trim_spaces( answer ); cpr_kill_prompt(); if( *answer == 'q' || *answer == 'Q') - return NULL; /* cancel */ + { + xfree (answer); + return NULL; /* cancel */ + } if( hint && !*answer ) n = hint; else if(!digitp( answer ) ) diff --git a/g10/tofu.c b/g10/tofu.c index f49083844..83786a08d 100644 --- a/g10/tofu.c +++ b/g10/tofu.c @@ -1687,6 +1687,8 @@ ask_about_binding (ctrl_t ctrl, GPGSQL_ARG_END); if (rc) { + sqlite3_free (sqerr); + sqerr = NULL; rc = gpg_error (GPG_ERR_GENERAL); break; } @@ -1972,6 +1974,7 @@ ask_about_binding (ctrl_t ctrl, else if (!response[0]) /* Default to unknown. Don't save it. */ { + xfree (response); tty_printf (_("Defaulting to unknown.\n")); *policy = TOFU_POLICY_UNKNOWN; break; @@ -1983,6 +1986,7 @@ ask_about_binding (ctrl_t ctrl, if (choice) { int c = ((size_t) choice - (size_t) choices) / 2; + xfree (response); switch (c) { diff --git a/g10/trustdb.c b/g10/trustdb.c index 43bce0769..9ef4644bf 100644 --- a/g10/trustdb.c +++ b/g10/trustdb.c @@ -1430,6 +1430,7 @@ ask_ownertrust (ctrl_t ctrl, u32 *kid, int minimum) { log_error (_("public key %s not found: %s\n"), keystr(kid), gpg_strerror (rc) ); + free_public_key (pk); return TRUST_UNKNOWN; } -- 2.30.2 From 0dabf0cffb1d67812c50a4f727398b59f93270a6 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 14:05:17 +0200 Subject: [PATCH GnuPG 04/19] sm: Avoid memory leaks and double double-free * sm/certcheck.c (extract_pss_params): Avoid double free * sm/decrypt.c (gpgsm_decrypt): goto leave instead of return * sm/encrypt.c (encrypt_dek): release s_pkey * sm/server.c (cmd_export): free list (do_listkeys): free lists -- Signed-off-by: Jakub Jelen --- sm/certcheck.c | 1 - sm/decrypt.c | 5 ++++- sm/encrypt.c | 1 + sm/server.c | 24 +++++++++++++++++++----- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/sm/certcheck.c b/sm/certcheck.c index fca45759b..f4db858c3 100644 --- a/sm/certcheck.c +++ b/sm/certcheck.c @@ -294,7 +294,6 @@ extract_pss_params (gcry_sexp_t s_sig, int *r_algo, unsigned int *r_saltlen) if (*r_saltlen < 20) { log_error ("length of PSS salt too short\n"); - gcry_sexp_release (s_sig); return gpg_error (GPG_ERR_DIGEST_ALGO); } if (!*r_algo) diff --git a/sm/decrypt.c b/sm/decrypt.c index aa91b370d..f7f91c466 100644 --- a/sm/decrypt.c +++ b/sm/decrypt.c @@ -755,7 +755,10 @@ gpgsm_decrypt (ctrl_t ctrl, int in_fd, estream_t out_fp) dfparm.mode = mode; dfparm.blklen = gcry_cipher_get_algo_blklen (algo); if (dfparm.blklen > sizeof (dfparm.helpblock)) - return gpg_error (GPG_ERR_BUG); + { + rc = gpg_error (GPG_ERR_BUG); + goto leave; + } rc = ksba_cms_get_content_enc_iv (cms, dfparm.iv, diff --git a/sm/encrypt.c b/sm/encrypt.c index 92ca341f5..ba2428e9a 100644 --- a/sm/encrypt.c +++ b/sm/encrypt.c @@ -473,6 +473,7 @@ encrypt_dek (const DEK dek, ksba_cert_t cert, int pk_algo, rc = encode_session_key (dek, &s_data); if (rc) { + gcry_sexp_release (s_pkey); log_error ("encode_session_key failed: %s\n", gpg_strerror (rc)); return rc; } diff --git a/sm/server.c b/sm/server.c index 874f0db89..871cc4b31 100644 --- a/sm/server.c +++ b/sm/server.c @@ -724,8 +724,13 @@ cmd_export (assuan_context_t ctx, char *line) if (opt_secret) { - if (!list || !*list->d) + if (!list) return set_error (GPG_ERR_NO_DATA, "No key given"); + if (!*list->d) + { + free_strlist (list); + return set_error (GPG_ERR_NO_DATA, "No key given"); + } if (list->next) return set_error (GPG_ERR_TOO_MANY, "Only one key allowed"); } @@ -1014,17 +1019,26 @@ do_listkeys (assuan_context_t ctx, char *line, int mode) int outfd = translate_sys2libc_fd (assuan_get_output_fd (ctx), 1); if ( outfd == -1 ) - return set_error (GPG_ERR_ASS_NO_OUTPUT, NULL); + { + free_strlist (list); + return set_error (GPG_ERR_ASS_NO_OUTPUT, NULL); + } fp = es_fdopen_nc (outfd, "w"); if (!fp) - return set_error (gpg_err_code_from_syserror (), "es_fdopen() failed"); + { + free_strlist (list); + return set_error (gpg_err_code_from_syserror (), "es_fdopen() failed"); + } } else { fp = es_fopencookie (ctx, "w", data_line_cookie_functions); if (!fp) - return set_error (GPG_ERR_ASS_GENERAL, - "error setting up a data stream"); + { + free_strlist (list); + return set_error (GPG_ERR_ASS_GENERAL, + "error setting up a data stream"); + } } ctrl->with_colons = 1; -- 2.30.2 From febbe77870b51e4e1158ae9efeaa0f3aad69a495 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 14:48:59 +0200 Subject: [PATCH GnuPG 05/19] tools: Avoid memory leak sfrom gpgspilt * tools/gpgsplit.c (write_part): free blob -- Signed-off-by: Jakub Jelen --- tools/gpgsplit.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/gpgsplit.c b/tools/gpgsplit.c index cc7bf8ef5..93458068c 100644 --- a/tools/gpgsplit.c +++ b/tools/gpgsplit.c @@ -620,6 +620,7 @@ write_part (FILE *fpin, unsigned long pktlen, } } + xfree (blob); goto ready; } -- 2.30.2 From 1cd048ba37786f46aabf3efdc3c245b75244dc26 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 19:19:59 +0200 Subject: [PATCH GnuPG 06/19] agent: Fix memory leaks * agent/call-daemon.c (daemon_start): free wctp * agent/call-scd.c (agent_card_pksign): return error instead of noop (card_keyinfo_cb): free keyinfo * agent/protect.c (agent_get_shadow_info_type): allocate only as a last action (agent_is_tpm2_key): Free buf -- Signed-off-by: Jakub Jelen --- agent/call-daemon.c | 2 ++ agent/call-scd.c | 4 +++- agent/protect.c | 17 +++++++++-------- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/agent/call-daemon.c b/agent/call-daemon.c index 144400875..3bf6bb793 100644 --- a/agent/call-daemon.c +++ b/agent/call-daemon.c @@ -512,6 +512,8 @@ daemon_start (enum daemon_type type, ctrl_t ctrl) log_error ("error spawning wait_child_thread: %s\n", strerror (err)); npth_attr_destroy (&tattr); } + else + xfree (wctp); } leave: diff --git a/agent/call-scd.c b/agent/call-scd.c index 3ede33c1d..f060541a3 100644 --- a/agent/call-scd.c +++ b/agent/call-scd.c @@ -487,7 +487,7 @@ agent_card_pksign (ctrl_t ctrl, /* FIXME: In the mdalgo case (INDATA,INDATALEN) might be long and * thus we can't convey it on a single Assuan line. */ if (!mdalgo) - gpg_error (GPG_ERR_NOT_IMPLEMENTED); + return gpg_error (GPG_ERR_NOT_IMPLEMENTED); if (indatalen*2 + 50 > DIM(line)) return unlock_scd (ctrl, gpg_error (GPG_ERR_GENERAL)); @@ -941,6 +941,7 @@ card_keyinfo_cb (void *opaque, const char *line) if (!keyinfo) { alloc_error: + xfree (keyinfo); if (!parm->error) parm->error = gpg_error_from_syserror (); return 0; @@ -952,6 +953,7 @@ card_keyinfo_cb (void *opaque, const char *line) if (n != 40) { parm_error: + xfree (keyinfo); if (!parm->error) parm->error = gpg_error (GPG_ERR_ASS_PARAMETER); return 0; diff --git a/agent/protect.c b/agent/protect.c index 50b10eb26..72169429d 100644 --- a/agent/protect.c +++ b/agent/protect.c @@ -1663,13 +1663,6 @@ agent_get_shadow_info_type (const unsigned char *shadowkey, n = snext (&s); if (!n) return gpg_error (GPG_ERR_INV_SEXP); - if (shadow_type) { - char *buf = xtrymalloc(n+1); - memcpy(buf, s, n); - buf[n] = '\0'; - *shadow_type = buf; - } - if (smatch (&s, n, "t1-v1") || smatch(&s, n, "tpm2-v1")) { if (*s != '(') @@ -1679,6 +1672,14 @@ agent_get_shadow_info_type (const unsigned char *shadowkey, } else return gpg_error (GPG_ERR_UNSUPPORTED_PROTOCOL); + + if (shadow_type) { + char *buf = xtrymalloc(n+1); + memcpy(buf, s, n); + buf[n] = '\0'; + *shadow_type = buf; + } + return 0; } @@ -1704,9 +1705,9 @@ agent_is_tpm2_key (gcry_sexp_t s_skey) return 0; err = agent_get_shadow_info_type (buf, NULL, &type); + xfree (buf); if (err) return 0; - xfree (buf); err = strcmp (type, "tpm2-v1") == 0; xfree (type); -- 2.30.2 From 9d206e1dfabb965e97723dd799d0f7b3be04116d Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 19:29:21 +0200 Subject: [PATCH GnuPG 07/19] common: Avoid double-free * common/name-value.c (do_nvc_parse): reset to null after ownership change -- Signed-off-by: Jakub Jelen --- common/name-value.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/name-value.c b/common/name-value.c index 0bd205b7d..39c3244e9 100644 --- a/common/name-value.c +++ b/common/name-value.c @@ -724,6 +724,7 @@ do_nvc_parse (nvc_t *result, int *errlinep, estream_t stream, if (raw_value) { err = _nvc_add (*result, name, NULL, raw_value, 1); + name = NULL; if (err) goto leave; } -- 2.30.2 From 33317744850d10a03ad4215a329a7d4bc3837234 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 19:48:31 +0200 Subject: [PATCH GnuPG 08/19] dirmgr: Avoid double free * dirmgr/http.c (http_prepare_redirect): Avoid double free * dirmgr/ocsp.c (check_signature): Initialize pointer -- Signed-off-by: Jakub Jelen --- dirmngr/http.c | 2 -- dirmngr/ocsp.c | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/dirmngr/http.c b/dirmngr/http.c index 74ce5f465..c662b1b95 100644 --- a/dirmngr/http.c +++ b/dirmngr/http.c @@ -3681,7 +3681,6 @@ http_prepare_redirect (http_redir_info_t *info, unsigned int status_code, if (!newurl) { err = gpg_error_from_syserror (); - http_release_parsed_uri (locuri); return err; } } @@ -3700,7 +3699,6 @@ http_prepare_redirect (http_redir_info_t *info, unsigned int status_code, if (!newurl) { err = gpg_error_from_syserror (); - http_release_parsed_uri (locuri); return err; } } diff --git a/dirmngr/ocsp.c b/dirmngr/ocsp.c index 6864f9854..6ec760d81 100644 --- a/dirmngr/ocsp.c +++ b/dirmngr/ocsp.c @@ -450,7 +450,7 @@ check_signature (ctrl_t ctrl, { gpg_error_t err; int algo, cert_idx; - gcry_sexp_t s_hash; + gcry_sexp_t s_hash = NULL; ksba_cert_t cert; const char *s; -- 2.30.2 From c87d40395b8f24426645cc491dca5cf910755395 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 20:05:48 +0200 Subject: [PATCH GnuPG 09/19] g10: Avoid memory leaks * g10/call-agent.c (card_keyinfo_cb): free keyinfo * g10/keyedit.c (menu_set_keyserver_url): properly enclose the block * g10/keygen.c (gen_card_key): free pk and pkt -- Signed-off-by: Jakub Jelen --- g10/call-agent.c | 2 ++ g10/keyedit.c | 8 +++++--- g10/keygen.c | 12 ++++++++++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/g10/call-agent.c b/g10/call-agent.c index 83355454a..c9cbcd4e5 100644 --- a/g10/call-agent.c +++ b/g10/call-agent.c @@ -1729,6 +1729,7 @@ card_keyinfo_cb (void *opaque, const char *line) if (!keyinfo) { alloc_error: + xfree (keyinfo); if (!parm->error) parm->error = gpg_error_from_syserror (); return 0; @@ -1740,6 +1741,7 @@ card_keyinfo_cb (void *opaque, const char *line) if (n != 40) { parm_error: + xfree (keyinfo); if (!parm->error) parm->error = gpg_error (GPG_ERR_ASS_PARAMETER); return 0; diff --git a/g10/keyedit.c b/g10/keyedit.c index 902741b5f..91731a271 100644 --- a/g10/keyedit.c +++ b/g10/keyedit.c @@ -5382,14 +5382,16 @@ menu_set_keyserver_url (ctrl_t ctrl, const char *url, kbnode_t pub_keyblock) uri ? _("Are you sure you want to replace it? (y/N) ") : _("Are you sure you want to delete it? (y/N) "))) - xfree (user); - continue; + { + xfree (user); + continue; + } } else if (uri == NULL) { /* There is no current keyserver URL, so there is no point in trying to un-set it. */ - xfree (user); + xfree (user); continue; } diff --git a/g10/keygen.c b/g10/keygen.c index f1e4d3638..82f6bb880 100644 --- a/g10/keygen.c +++ b/g10/keygen.c @@ -6140,12 +6140,20 @@ gen_card_key (int keyno, int algo, int is_primary, kbnode_t pub_root, the self-signatures. */ err = agent_readkey (NULL, 1, keyid, &public); if (err) - return err; + { + xfree (pkt); + xfree (pk); + return err; + } err = gcry_sexp_sscan (&s_key, NULL, public, gcry_sexp_canon_len (public, 0, NULL, NULL)); xfree (public); if (err) - return err; + { + xfree (pkt); + xfree (pk); + return err; + } if (algo == PUBKEY_ALGO_RSA) err = key_from_sexp (pk->pkey, s_key, "public-key", "ne"); -- 2.30.2 From bb6e1e13d9440816c60013a50d426b7ccd6c0288 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Mon, 12 Apr 2021 21:59:17 +0200 Subject: [PATCH GnuPG 10/19] kbx: Avoid uninitialized read * kbx/kbx-client-util.c (datastream_thread): Initialize pointer * kbx/keybox-dump.c (_keybox_dump_cut_records): free blob * kbx/kbxserver.c (kbxd_start_command_handler): do not free passed ctrl * kbx/keyboxd.c (check_own_socket): free sockname -- Signed-off-by: Jakub Jelen --- kbx/kbx-client-util.c | 2 +- kbx/kbxserver.c | 1 - kbx/keybox-dump.c | 4 +++- kbx/keyboxd.c | 5 ++++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/kbx/kbx-client-util.c b/kbx/kbx-client-util.c index bd71cf2ba..07370319b 100644 --- a/kbx/kbx-client-util.c +++ b/kbx/kbx-client-util.c @@ -176,7 +176,7 @@ datastream_thread (void *arg) int rc; unsigned char lenbuf[4]; size_t nread, datalen; - char *data, *tmpdata; + char *data = NULL, *tmpdata; /* log_debug ("%s: started\n", __func__); */ while (kcd->fp) diff --git a/kbx/kbxserver.c b/kbx/kbxserver.c index 55b478586..0b76cde31 100644 --- a/kbx/kbxserver.c +++ b/kbx/kbxserver.c @@ -844,7 +844,6 @@ kbxd_start_command_handler (ctrl_t ctrl, gnupg_fd_t fd, unsigned int session_id) { log_error (_("can't allocate control structure: %s\n"), gpg_strerror (gpg_error_from_syserror ())); - xfree (ctrl); return; } ctrl->server_local->client_pid = ASSUAN_INVALID_PID; diff --git a/kbx/keybox-dump.c b/kbx/keybox-dump.c index 3e66b72a1..38608ceaa 100644 --- a/kbx/keybox-dump.c +++ b/kbx/keybox-dump.c @@ -881,7 +881,7 @@ _keybox_dump_cut_records (const char *filename, unsigned long from, unsigned long to, FILE *outfp) { estream_t fp; - KEYBOXBLOB blob; + KEYBOXBLOB blob = NULL; int rc; unsigned long recno = 0; @@ -902,6 +902,7 @@ _keybox_dump_cut_records (const char *filename, unsigned long from, } } _keybox_release_blob (blob); + blob = NULL; recno++; } if (rc == -1) @@ -909,6 +910,7 @@ _keybox_dump_cut_records (const char *filename, unsigned long from, if (rc) fprintf (stderr, "error reading '%s': %s\n", filename, gpg_strerror (rc)); leave: + _keybox_release_blob (blob); if (fp != es_stdin) es_fclose (fp); return rc; diff --git a/kbx/keyboxd.c b/kbx/keyboxd.c index 76a0694a4..3f759e6f7 100644 --- a/kbx/keyboxd.c +++ b/kbx/keyboxd.c @@ -1795,7 +1795,10 @@ check_own_socket (void) err = npth_attr_init (&tattr); if (err) - return; + { + xfree (sockname); + return; + } npth_attr_setdetachstate (&tattr, NPTH_CREATE_DETACHED); err = npth_create (&thread, &tattr, check_own_socket_thread, sockname); if (err) -- 2.30.2 From 7f54495586491b18b5e1088ecf5538c0343f90e7 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 13 Apr 2021 14:02:18 +0200 Subject: [PATCH GnuPG 11/19] scd: avoid memory leaks * scd/app-p15.c (send_certinfo): free labelbuf (do_sign): goto leave instead of return * scd/app-piv.c (do_sign): goto leave instead of return, fix typo in variable name, avoid using uninitialized variables * scd/command.c (cmd_genkey): goto leave instead of return -- Signed-off-by: Jakub Jelen --- scd/app-p15.c | 5 +++-- scd/app-piv.c | 6 +++--- scd/command.c | 10 ++++++++-- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/scd/app-p15.c b/scd/app-p15.c index 90f6b4c99..9eeeed960 100644 --- a/scd/app-p15.c +++ b/scd/app-p15.c @@ -3851,6 +3851,7 @@ send_certinfo (app_t app, ctrl_t ctrl, const char *certtype, labelbuf, strlen (labelbuf), NULL, (size_t)0); xfree (buf); + xfree (labelbuf); } return 0; } @@ -5461,7 +5462,7 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo, if (err) { log_error ("p15: MSE failed: %s\n", gpg_strerror (err)); - return err; + goto leave; } /* Now that we have all the information available run the actual PIN @@ -5500,7 +5501,7 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo, if (err) { log_error ("p15: MSE failed: %s\n", gpg_strerror (err)); - return err; + goto leave; } if (prkdf->keyalgo == GCRY_PK_RSA && prkdf->keynbits > 2048) diff --git a/scd/app-piv.c b/scd/app-piv.c index ead1b1974..143cc047a 100644 --- a/scd/app-piv.c +++ b/scd/app-piv.c @@ -2175,7 +2175,7 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo, unsigned char oidbuf[64]; size_t oidbuflen; unsigned char *outdata = NULL; - size_t outdatalen; + size_t outdatalen = 0; const unsigned char *s; size_t n; int keyref, mechanism; @@ -2357,7 +2357,7 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo, /* Now verify the Application PIN. */ err = verify_chv (app, ctrl, 0x80, force_verify, pincb, pincb_arg); if (err) - return err; + goto leave; /* Build the Dynamic Authentication Template. */ err = concat_tlv_list (0, &apdudata, &apdudatalen, @@ -2403,7 +2403,7 @@ do_sign (app_t app, ctrl_t ctrl, const char *keyidstr, int hashalgo, goto bad_der; log_assert (n >= (rval-s)+rlen); sval = find_tlv (rval+rlen, n-((rval-s)+rlen), 0x02, &slen); - if (!rval) + if (!sval) goto bad_der; rlenx = slenx = 0; if (rlen > slen) diff --git a/scd/command.c b/scd/command.c index 11d61648b..cb0dd379a 100644 --- a/scd/command.c +++ b/scd/command.c @@ -1438,7 +1438,10 @@ cmd_genkey (assuan_context_t ctx, char *line) line = skip_options (line); if (!*line) - return set_error (GPG_ERR_ASS_PARAMETER, "no key number given"); + { + err = set_error (GPG_ERR_ASS_PARAMETER, "no key number given"); + goto leave; + } keyref = line; while (*line && !spacep (line)) line++; @@ -1448,7 +1451,10 @@ cmd_genkey (assuan_context_t ctx, char *line) goto leave; if (!ctrl->card_ctx) - return gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); + { + err = gpg_error (GPG_ERR_UNSUPPORTED_OPERATION); + goto leave; + } keyref = keyref_buffer = xtrystrdup (keyref); if (!keyref) -- 2.30.2 From 3b2d9059b95ddb95a9e9fbaceb2f17c8be31d229 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 13 Apr 2021 14:50:13 +0200 Subject: [PATCH GnuPG 12/19] tools: Intialize pointer to avoid double free * tools/gpg-card.c (cmd_salut): Initialize data pointer -- Signed-off-by: Jakub Jelen --- tools/gpg-card.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/gpg-card.c b/tools/gpg-card.c index 1889fb45c..e84d2fbb0 100644 --- a/tools/gpg-card.c +++ b/tools/gpg-card.c @@ -1785,6 +1785,7 @@ cmd_salut (card_info_t info, const char *argstr) { tty_printf (_("Error: invalid response.\n")); xfree (data); + data = NULL; goto again; } } -- 2.30.2 From 7c8048b686a6e811d0b24febf3c5e2528e7881f1 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 13 Apr 2021 16:23:31 +0200 Subject: [PATCH GnuPG 14/19] dirmgr: Avoid memory leaks * dirmngr/domaininfo.c (insert_or_update): free di_new -- Signed-off-by: Jakub Jelen --- dirmngr/domaininfo.c | 1 + 1 file changed, 1 insertion(+) diff --git a/dirmngr/domaininfo.c b/dirmngr/domaininfo.c index b41aef366..87782b4b1 100644 --- a/dirmngr/domaininfo.c +++ b/dirmngr/domaininfo.c @@ -193,6 +193,7 @@ insert_or_update (const char *domain, log_error ("domaininfo: error allocating helper array: %s\n", gpg_strerror (gpg_err_code_from_syserror ())); drop_extra = bucket; + xfree (di_new); goto leave; } narray = 0; -- 2.30.2 From ab3b8c53993b3305088efde756a44bac6e6492d4 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 13 Apr 2021 16:34:40 +0200 Subject: [PATCH GnuPG 15/19] scd: Avoid memory leaks and uninitialized memory * scd/app-piv.c (do_decipher): goto leave, initialize outdatalen -- Signed-off-by: Jakub Jelen --- scd/app-piv.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scd/app-piv.c b/scd/app-piv.c index 143cc047a..94257f0ee 100644 --- a/scd/app-piv.c +++ b/scd/app-piv.c @@ -2483,7 +2483,7 @@ do_decipher (app_t app, ctrl_t ctrl, const char *keyidstr, gpg_error_t err; data_object_t dobj; unsigned char *outdata = NULL; - size_t outdatalen; + size_t outdatalen = 0; const unsigned char *s; size_t n; int keyref, mechanism; @@ -2582,7 +2582,7 @@ do_decipher (app_t app, ctrl_t ctrl, const char *keyidstr, /* Now verify the Application PIN. */ err = verify_chv (app, ctrl, 0x80, 0, pincb, pincb_arg); if (err) - return err; + goto leave; /* Build the Dynamic Authentication Template. */ err = concat_tlv_list (0, &apdudata, &apdudatalen, -- 2.30.2 From f182bf91443618323e34261039045a6bde269be5 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Tue, 13 Apr 2021 16:44:48 +0200 Subject: [PATCH GnuPG 16/19] tools: Avoid memory leaks * tools/wks-util.c (wks_cmd_print_wkd_url): Free addrspec on error (wks_cmd_print_wkd_hash): Free addrspec on error -- Signed-off-by: Jakub Jelen --- tools/wks-util.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tools/wks-util.c b/tools/wks-util.c index 516c7fe00..38dd194ff 100644 --- a/tools/wks-util.c +++ b/tools/wks-util.c @@ -1192,11 +1192,14 @@ gpg_error_t wks_cmd_print_wkd_hash (const char *userid) { gpg_error_t err; - char *addrspec, *fname; + char *addrspec = NULL, *fname; err = wks_fname_from_userid (userid, 1, &fname, &addrspec); if (err) - return err; + { + xfree (addrspec); + return err; + } es_printf ("%s %s\n", fname, addrspec); @@ -1211,12 +1214,15 @@ gpg_error_t wks_cmd_print_wkd_url (const char *userid) { gpg_error_t err; - char *addrspec, *fname; + char *addrspec = NULL, *fname; char *domain; err = wks_fname_from_userid (userid, 1, &fname, &addrspec); if (err) - return err; + { + xfree (addrspec); + return err; + } domain = strchr (addrspec, '@'); if (domain) -- 2.30.2 From 600fabd8268c765d45d48873e7a8610e6dae0966 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 14 Apr 2021 15:59:12 +0200 Subject: [PATCH GnuPG 17/19] scd: Use the same allocator to free memory * scd/command.c (cmd_getinfo): Use free instead of gcry_free to match the original allocator -- Signed-off-by: Jakub Jelen --- scd/command.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scd/command.c b/scd/command.c index cb0dd379a..9d85c5a41 100644 --- a/scd/command.c +++ b/scd/command.c @@ -1832,7 +1832,8 @@ cmd_getinfo (assuan_context_t ctx, char *line) rc = assuan_send_data (ctx, p, strlen (p)); else rc = gpg_error (GPG_ERR_NO_DATA); - xfree (p); + /* allocated by scd/ccid-driver.c which is not using x*alloc/gcry_* */ + free (p); } else if (!strcmp (line, "deny_admin")) rc = opt.allow_admin? gpg_error (GPG_ERR_GENERAL) : 0; -- 2.30.2 From f45f023495cb9947b2c31b5782a4063ad317c34c Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 14 Apr 2021 18:46:48 +0200 Subject: [PATCH GnuPG 18/19] common: Mark identical branches as intential * common/tlv-builder.c (get_tlv_length): Mark identical branches as inentional for coverity -- Signed-off-by: Jakub Jelen --- common/tlv-builder.c | 1 + 1 file changed, 1 insertion(+) diff --git a/common/tlv-builder.c b/common/tlv-builder.c index 3b644ca24..59e2691e0 100644 --- a/common/tlv-builder.c +++ b/common/tlv-builder.c @@ -350,6 +350,7 @@ get_tlv_length (int class, int tag, int constructed, size_t length) (void)constructed; /* Not used, but passed for uniformity of such calls. */ + /* coverity[identical_branches] */ if (tag < 0x1f) { buflen++; -- 2.30.2 From a94b0deab7c2ece2e512f87a52142454354d77b5 Mon Sep 17 00:00:00 2001 From: Jakub Jelen Date: Wed, 14 Apr 2021 18:49:03 +0200 Subject: [PATCH GnuPG 19/19] g10: Do not allocate memory when we can't return it * g10/keyid.c (fpr20_from_pk): Do not allocate memory when we can't return it -- Signed-off-by: Jakub Jelen --- g10/keyid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/g10/keyid.c b/g10/keyid.c index 522cc9cda..f1af2fd90 100644 --- a/g10/keyid.c +++ b/g10/keyid.c @@ -899,7 +899,7 @@ fpr20_from_pk (PKT_public_key *pk, byte array[20]) compute_fingerprint (pk); if (!array) - array = xmalloc (pk->fprlen); + return; if (pk->fprlen == 32) /* v5 fingerprint */ { -- 2.30.2