From f312c4df7c8b8bbcf4111a566d8acd0ec73e7592 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 18:54:19 +0200 Subject: [PATCH 01/20] Don't let do_encryption() call gcryp_create_nonce() and gcry_cipher_setiv() if we failed to allocate space for the iv. --- agent/protect.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git agent/protect.c agent/protect.c index 8b022ec..3b44705 100644 --- agent/protect.c +++ agent/protect.c @@ -175,9 +175,14 @@ do_encryption (const unsigned char *protbegin, size_t protlen, /* Allocate random bytes to be used as IV, padding and s2k salt. */ iv = xtrymalloc (blklen*2+8); if (!iv) - rc = gpg_error (GPG_ERR_ENOMEM); - gcry_create_nonce (iv, blklen*2+8); - rc = gcry_cipher_setiv (hd, iv, blklen); + { + rc = gpg_error (GPG_ERR_ENOMEM); + } + else + { + gcry_create_nonce (iv, blklen*2+8); + rc = gcry_cipher_setiv (hd, iv, blklen); + } } if (!rc) { -- 1.6.3.2 From a02fe4e981ad7a874e307f1b7b9491f207b77dec Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 17:50:51 +0200 Subject: [PATCH 02/20] If parse_sexp() in get_rsa_pk_from_canon_sexp's loop condition fails, return err. This is already done everywhere else in the function, and I get the impression that not doing it could cause get_rsa_pk_from_canon_sexp() to return an incorrect return code if the loop expression doesn't fail right away. --- common/sexputil.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git common/sexputil.c common/sexputil.c index 7c6cb6a..7360881 100644 --- common/sexputil.c +++ common/sexputil.c @@ -377,6 +377,9 @@ get_rsa_pk_from_canon_sexp (const unsigned char *keydata, size_t keydatalen, return err; } + if (err) + return err; + if (!rsa_n || !rsa_n_len || !rsa_e || !rsa_e_len) return gpg_error (GPG_ERR_BAD_PUBKEY); -- 1.6.3.2 From f0368d260ae70a6368410e5471d20fee0b76c5bc Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 18:25:17 +0200 Subject: [PATCH 03/20] In mpi_read(), make sure jumping to 'leave:' early on doesn't cause nread to be read uninitialized. --- g10/parse-packet.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git g10/parse-packet.c g10/parse-packet.c index a86e549..16ca751 100644 --- g10/parse-packet.c +++ g10/parse-packet.c @@ -112,7 +112,7 @@ mpi_read (iobuf_t inp, unsigned int *ret_nread, int secure) int c, c1, c2, i; unsigned int nbits, nbytes; - size_t nread; + size_t nread = 0; gcry_mpi_t a = NULL; byte *buf = NULL; byte *p; -- 1.6.3.2 From f95596a7980244a5559fbb45726aacb3af59aba0 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 20:17:13 +0200 Subject: [PATCH 04/20] In encrypt_dek(), don't ignore gcry_pk_encrypt()'s return code. --- sm/encrypt.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git sm/encrypt.c sm/encrypt.c index a526a64..a67a4b7 100644 --- sm/encrypt.c +++ sm/encrypt.c @@ -208,7 +208,8 @@ encrypt_dek (const DEK dek, ksba_cert_t cert, unsigned char **encval) gcry_sexp_release (s_pkey); /* Reformat it. */ - rc = make_canon_sexp (s_ciph, encval, NULL); + if (!rc) + rc = make_canon_sexp (s_ciph, encval, NULL); gcry_sexp_release (s_ciph); return rc; } -- 1.6.3.2 From f80eccc9c9b6b83095a7ca517ff65af3e988e156 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 18:44:42 +0200 Subject: [PATCH 05/20] I don't see a reason why agent_protect_and_store() shouldn't signal store_key() failures. --- agent/genkey.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git agent/genkey.c agent/genkey.c index 4bf0dcd..d862963 100644 --- agent/genkey.c +++ agent/genkey.c @@ -480,5 +480,5 @@ agent_protect_and_store (ctrl_t ctrl, gcry_sexp_t s_skey) rc = store_key (s_skey, pi? pi->pin:NULL, 1); xfree (pi); - return 0; + return rc; } -- 1.6.3.2 From 128ceafc19b9961cf841326f621649eef43070b7 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 18:32:14 +0200 Subject: [PATCH 06/20] Make sure reate_request() returns gpg_error (GPG_ERR_BUG) if "libksba did not return a proper S-Exp" --- sm/certreqgen.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git sm/certreqgen.c sm/certreqgen.c index ca791aa..59e6679 100644 --- sm/certreqgen.c +++ sm/certreqgen.c @@ -769,7 +769,7 @@ create_request (ctrl_t ctrl, if (!n) { log_error ("libksba did not return a proper S-Exp\n"); - err = gpg_error (GPG_ERR_BUG); + rc = gpg_error (GPG_ERR_BUG); goto leave; } rc = gcry_sexp_sscan (&s_pkey, NULL, (const char*)public, n); -- 1.6.3.2 From e8fce099777ea8352feca31b48b51cbd91be0e53 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 21:47:31 +0200 Subject: [PATCH 07/20] Fix what looks like a memory leak in gen_revoke. The allocated memory doesn't seem to be used anywhere. --- g10/revoke.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git g10/revoke.c g10/revoke.c index cc66dfc..cce6d69 100644 --- g10/revoke.c +++ g10/revoke.c @@ -489,8 +489,6 @@ gen_revoke( const char *uname ) keyid_from_sk( sk, sk_keyid ); print_seckey_info (sk); - pk = xmalloc_clear( sizeof *pk ); - /* FIXME: We should get the public key direct from the secret one */ pub_keyblock=get_pubkeyblock(sk_keyid); -- 1.6.3.2 From 86987d83ba983bf9bbf01d0277f3e479d549b7d9 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 20:32:19 +0200 Subject: [PATCH 08/20] Fix NULL pointer dereferences in es_read_line() and readline(). --- common/estream.c | 4 +++- common/xreadline.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git common/estream.c common/estream.c index 214c2ff..840422c 100644 --- common/estream.c +++ common/estream.c @@ -2797,7 +2797,9 @@ es_read_line (estream_t stream, { int save_errno = errno; mem_free (buffer); - *length_of_buffer = *max_length = 0; + *length_of_buffer = 0; + if (max_length) + *max_length = 0; ESTREAM_UNLOCK (stream); errno = save_errno; return -1; diff --git common/xreadline.c common/xreadline.c index ab43c29..8ca72b7 100644 --- common/xreadline.c +++ common/xreadline.c @@ -95,7 +95,9 @@ read_line (FILE *fp, { int save_errno = errno; xfree (buffer); - *length_of_buffer = *max_length = 0; + *length_of_buffer = 0; + if (max_length) + *max_length = 0; errno = save_errno; return -1; } -- 1.6.3.2 From 72120620a1f7e2f319589c449abcaf3c42448901 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 20:54:36 +0200 Subject: [PATCH 09/20] Fix a NULL pointer dereference in passphrase_to_dek_ext(). --- g10/passphrase.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git g10/passphrase.c g10/passphrase.c index 3742738..773f114 100644 --- g10/passphrase.c +++ g10/passphrase.c @@ -600,7 +600,7 @@ passphrase_to_dek_ext (u32 *keyid, int pubkey_algo, get_last_passphrase(). */ dek = xmalloc_secure_clear ( sizeof *dek ); dek->algo = cipher_algo; - if ( !*pw && (mode == 2 || mode == 4)) + if ((!pw || !*pw) && (mode == 2 || mode == 4)) dek->keylen = 0; else hash_passphrase (dek, pw, s2k); -- 1.6.3.2 From 27326b3f68035b792384315d836f4d98938132ba Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 21:11:37 +0200 Subject: [PATCH 10/20] Fix a NULL pointer dereference in send_key() if allocating memory for modlist failed. --- keyserver/gpgkeys_ldap.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git keyserver/gpgkeys_ldap.c keyserver/gpgkeys_ldap.c index 3fed8c5..9a47347 100644 --- keyserver/gpgkeys_ldap.c +++ keyserver/gpgkeys_ldap.c @@ -772,7 +772,7 @@ send_key(int *r_eof) fail: /* Unwind and free the whole modlist structure */ - for(ml=modlist;*ml;ml++) + for(ml=modlist;ml&&*ml;ml++) { free_mod_values(*ml); free(*ml); -- 1.6.3.2 From bdace71901c9d20fdc31af794790b853ed91d778 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 21:31:53 +0200 Subject: [PATCH 11/20] Fix a dead assignment in ec_func_mem_write. XXX: This is a just a guess. --- common/estream.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git common/estream.c common/estream.c index 840422c..a6956ba 100644 --- common/estream.c +++ common/estream.c @@ -464,7 +464,7 @@ es_func_mem_write (void *cookie, const void *buffer, size_t size) newsize = mem_cookie->memory_size + mem_cookie->block_size; - newsize = mem_cookie->offset + size; + newsize += mem_cookie->offset + size; if (newsize < mem_cookie->offset) { errno = EINVAL; -- 1.6.3.2 From 4534df038ad096c957d56dc7b406dca24f0b0234 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 20:05:17 +0200 Subject: [PATCH 12/20] Fix a dead assignment in gpgsm_sign(). --- sm/sign.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git sm/sign.c sm/sign.c index 446cd37..0569052 100644 --- sm/sign.c +++ sm/sign.c @@ -403,7 +403,7 @@ gpgsm_sign (ctrl_t ctrl, certlist_t signerlist, log_info ("user requested hash algorithm %d\n", opt.forced_digest_algo); for (i=0, cl=signerlist; cl; cl = cl->next, i++) { - const char *oid = ksba_cert_get_digest_algo (cl->cert); + const char *oid; if (opt.forced_digest_algo) { -- 1.6.3.2 From 7e2992be8fa9de05ed376baaba9c6fad75e48239 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 18:44:47 +0200 Subject: [PATCH 13/20] Let es_write_sanitized_utf8_buffer() return an error if es_fputs() fails. --- common/estream.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git common/estream.c common/estream.c index a6956ba..c930bd7 100644 --- common/estream.c +++ common/estream.c @@ -3205,7 +3205,7 @@ es_write_sanitized_utf8_buffer (estream_t stream, *bytes_written = strlen (buf); ret = es_fputs (buf, stream); xfree (buf); - return i; + return ret ? ret : i; } else return es_write_sanitized (stream, p, length, delimiters, bytes_written); -- 1.6.3.2 From ef9cad0e8311fadfa6fe209a03d30523202349ff Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 20:24:40 +0200 Subject: [PATCH 14/20] Remove a pointless line in unhexify_fpr() --- g10/call-agent.c | 1 - sm/call-dirmngr.c | 1 - 2 files changed, 0 insertions(+), 2 deletions(-) diff --git g10/call-agent.c g10/call-agent.c index cd58b90..0590514 100644 --- g10/call-agent.c +++ g10/call-agent.c @@ -132,7 +132,6 @@ unhexify_fpr (const char *hexstr, unsigned char *fpr) ; if (*s || (n != 40)) return 0; /* no fingerprint (invalid or wrong length). */ - n /= 2; for (s=hexstr, n=0; *s; s += 2, n++) fpr[n] = xtoi_2 (s); return 1; /* okay */ diff --git sm/call-dirmngr.c sm/call-dirmngr.c index 914fdd0..5bd47d2 100644 --- sm/call-dirmngr.c +++ sm/call-dirmngr.c @@ -497,7 +497,6 @@ unhexify_fpr (const char *hexstr, unsigned char *fpr) ; if (*s || (n != 40)) return 0; /* no fingerprint (invalid or wrong length). */ - n /= 2; for (s=hexstr, n=0; *s; s += 2, n++) fpr[n] = xtoi_2 (s); return 1; /* okay */ -- 1.6.3.2 From fe69040c181997f256ae8a0032d1a518208e4999 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 21:37:43 +0200 Subject: [PATCH 15/20] Remove write-only variable need_words in keybox_search. --- kbx/keybox-search.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git kbx/keybox-search.c kbx/keybox-search.c index 08b59e6..c5758b9 100644 --- kbx/keybox-search.c +++ kbx/keybox-search.c @@ -694,7 +694,7 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc) { int rc; size_t n; - int need_words, any_skip; + int any_skip; KEYBOXBLOB blob = NULL; struct sn_array_s *sn_array = NULL; @@ -714,13 +714,12 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc) return -1; /* still EOF */ /* figure out what information we need */ - need_words = any_skip = 0; + any_skip = 0; for (n=0; n < ndesc; n++) { switch (desc[n].mode) { case KEYDB_SEARCH_MODE_WORDS: - need_words = 1; break; case KEYDB_SEARCH_MODE_FIRST: /* always restart the search in this mode */ -- 1.6.3.2 From 5223db7ab6983f986acd4b7aa10f290cd60a73e7 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 18:17:55 +0200 Subject: [PATCH 16/20] Remove write-only variable did_checkpin in card_edit(). --- g10/card-util.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-) diff --git g10/card-util.c g10/card-util.c index 9295a17..c570373 100644 --- g10/card-util.c +++ g10/card-util.c @@ -1600,7 +1600,7 @@ card_edit (strlist_t commands) int have_commands = !!commands; int redisplay = 1; char *answer = NULL; - int did_checkpin = 0, allow_admin=0; + int allow_admin = 0; char serialnobuf[50]; @@ -1812,12 +1812,10 @@ card_edit (strlist_t commands) case cmdPASSWD: change_pin (0, allow_admin); - did_checkpin = 0; /* Need to reset it of course. */ break; case cmdUNBLOCK: change_pin (1, allow_admin); - did_checkpin = 0; /* Need to reset it of course. */ break; case cmdQUIT: -- 1.6.3.2 From 01481ebb81957ca86a355f03f208eeb847d68d84 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Mon, 22 Jun 2009 21:51:50 +0200 Subject: [PATCH 17/20] Fix a dead assignment in keyring_search. --- g10/keyring.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git g10/keyring.c g10/keyring.c index 2c89431..dc6d6c6 100644 --- g10/keyring.c +++ g10/keyring.c @@ -997,7 +997,6 @@ keyring_search (KEYRING_HANDLE hd, KEYDB_SEARCH_DESC *desc, hd->word_match.name = xstrdup (name); hd->word_match.pattern = prepare_word_match (name); } - name = hd->word_match.pattern; } init_packet(&pkt); -- 1.6.3.2 From 0b57a1ee5042a08d24f0f079ded9c5b188031c34 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 17:56:13 +0200 Subject: [PATCH 18/20] Fix a pointless assignment in menu_select_key(). --- g10/keyedit.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git g10/keyedit.c g10/keyedit.c index b0d59f6..05d1b5e 100644 --- g10/keyedit.c +++ g10/keyedit.c @@ -4543,7 +4543,7 @@ menu_select_key( KBNODE keyblock, int idx ) } } else { /* reset all */ - for( i=0, node = keyblock; node; node = node->next ) { + for( node = keyblock; node; node = node->next ) { if( node->pkt->pkttype == PKT_PUBLIC_SUBKEY || node->pkt->pkttype == PKT_SECRET_SUBKEY ) node->flag &= ~NODFLG_SELKEY; -- 1.6.3.2 From 490912fe69d16915ac1e3c6c80f8958d357129e4 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 17:58:12 +0200 Subject: [PATCH 19/20] Fix a pointless assignment in menu_select_uid(). --- g10/keyedit.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git g10/keyedit.c g10/keyedit.c index 05d1b5e..8541660 100644 --- g10/keyedit.c +++ g10/keyedit.c @@ -4458,7 +4458,7 @@ menu_select_uid( KBNODE keyblock, int idx ) } } else { /* reset all */ - for( i=0, node = keyblock; node; node = node->next ) { + for( node = keyblock; node; node = node->next ) { if( node->pkt->pkttype == PKT_USER_ID ) node->flag &= ~NODFLG_SELUID; } -- 1.6.3.2 From 5efd9605a4f5c93da896db1495710c644819cc04 Mon Sep 17 00:00:00 2001 From: Fabian Keil Date: Tue, 23 Jun 2009 18:24:56 +0200 Subject: [PATCH 20/20] n isn't used in the second for loop in pattern_from_strlist, so there's no need to reset it there. --- sm/call-dirmngr.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git sm/call-dirmngr.c sm/call-dirmngr.c index 5bd47d2..9098ab5 100644 --- sm/call-dirmngr.c +++ sm/call-dirmngr.c @@ -746,7 +746,7 @@ pattern_from_strlist (strlist_t names) if (!pattern) return NULL; - for (n=0, sl=names; sl; sl = sl->next) + for (sl=names; sl; sl = sl->next) { for (s=sl->d; *s; s++) { -- 1.6.3.2