From 2e0ce7d97f0de998cdf8e95e17ce169b7cae91cd Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 24 Jun 2009 14:03:09 +0000 Subject: [PATCH] Fixed a bunch of little bugs as reported by Fabian Keil. Still one problem left; marked with a gcc #warning. --- THANKS | 1 + agent/ChangeLog | 6 ++++++ agent/genkey.c | 2 +- agent/protect.c | 7 +++++-- common/ChangeLog | 10 ++++++++++ common/estream.c | 12 +++++++----- common/sexputil.c | 3 +++ common/xreadline.c | 4 +++- g10/ChangeLog | 12 ++++++++++++ g10/call-agent.c | 1 - g10/card-util.c | 4 +--- g10/keyedit.c | 4 ++-- g10/keyring.c | 2 +- g10/parse-packet.c | 2 +- g10/passphrase.c | 2 +- g10/revoke.c | 2 -- kbx/keybox-search.c | 2 ++ keyserver/ChangeLog | 5 +++++ keyserver/gpgkeys_ldap.c | 14 ++++++++------ sm/ChangeLog | 7 +++++++ sm/call-dirmngr.c | 2 +- sm/certreqgen.c | 2 +- sm/sign.c | 2 +- 23 files changed, 79 insertions(+), 29 deletions(-) diff --git a/THANKS b/THANKS index b27952893..589ab257a 100644 --- a/THANKS +++ b/THANKS @@ -75,6 +75,7 @@ Edwin Woudt edwin at woudt.nl Enzo Michelangeli em at MailAndNews.com Ernst Molitor ernst.molitor at uni-bonn.de Evgeny Legerov +Fabian Keil fk at fabiankeil de Fabio Coatti cova at ferrara.linux.it Felix von Leitner leitner at amdiv.de fish stiqz fish at analog.org diff --git a/agent/ChangeLog b/agent/ChangeLog index 329990951..4829ff2d4 100644 --- a/agent/ChangeLog +++ b/agent/ChangeLog @@ -1,3 +1,9 @@ +2009-06-24 Werner Koch + + * genkey.c (agent_protect_and_store): Return RC and not 0. + * protect.c (do_encryption): Fix ignored error code from malloc. + Reported by Fabian Keil. + 2009-06-17 Werner Koch * call-pinentry.c (agent_get_confirmation): Add arg WITH_CANCEL. diff --git a/agent/genkey.c b/agent/genkey.c index 4bf0dcdcb..d86296390 100644 --- a/agent/genkey.c +++ b/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; } diff --git a/agent/protect.c b/agent/protect.c index 8b022ecfb..d6457ad2a 100644 --- a/agent/protect.c +++ b/agent/protect.c @@ -176,8 +176,11 @@ do_encryption (const unsigned char *protbegin, size_t protlen, 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); + else + { + gcry_create_nonce (iv, blklen*2+8); + rc = gcry_cipher_setiv (hd, iv, blklen); + } } if (!rc) { diff --git a/common/ChangeLog b/common/ChangeLog index 50669612e..dd2db13cb 100644 --- a/common/ChangeLog +++ b/common/ChangeLog @@ -1,3 +1,13 @@ +2009-06-24 Werner Koch + + * estream.c (es_read_line): In the malloc error case, set + MAX_LENGTH to 0 only if requested. + * xreadline.c (read_line): Ditto. + * estream.c (es_write_sanitized_utf8_buffer): Pass on error from + es_fputs. + * sexputil.c (get_rsa_pk_from_canon_sexp): Check for error after + the loop. Reported by Fabian Keil. + 2009-06-03 Werner Koch * estream.c (es_convert_mode): Rewrite and support the "x" flag. diff --git a/common/estream.c b/common/estream.c index 214c2ff7d..d3557750d 100644 --- a/common/estream.c +++ b/common/estream.c @@ -450,8 +450,8 @@ es_func_mem_write (void *cookie, const void *buffer, size_t size) if (!mem_cookie->flags.grow) { - /* We are not alloew to grow, thus limit the size to the left - space. FIXME: Does the grow flag an its semtics make sense + /* We are not allowed to grow, thus limit the size to the left + space. FIXME: Does the grow flag and its sematics make sense at all? */ if (size > mem_cookie->memory_size - mem_cookie->offset) size = mem_cookie->memory_size - mem_cookie->offset; @@ -463,7 +463,7 @@ es_func_mem_write (void *cookie, const void *buffer, size_t size) size_t newsize; newsize = mem_cookie->memory_size + mem_cookie->block_size; - +#warning READ the code and see how it should work newsize = mem_cookie->offset + size; if (newsize < mem_cookie->offset) { @@ -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; @@ -3203,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 rt == EOF? ret : (int)i; } else return es_write_sanitized (stream, p, length, delimiters, bytes_written); diff --git a/common/sexputil.c b/common/sexputil.c index 7c6cb6af5..73608816d 100644 --- a/common/sexputil.c +++ b/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); diff --git a/common/xreadline.c b/common/xreadline.c index ab43c292a..8ca72b75f 100644 --- a/common/xreadline.c +++ b/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; } diff --git a/g10/ChangeLog b/g10/ChangeLog index e961921a1..fcf759041 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,15 @@ +2009-06-24 Werner Koch + + * keyedit.c (menu_select_key): Renmove dead assign to I. + (menu_select_uid): Ditto. + * keyring.c (keyring_search): Remove dead assign to NAME. + * card-util.c (card_edit): Remove useless DID_CHECKPIN. + * call-agent.c (unhexify_fpr): Remove dead op on N. + * passphrase.c (passphrase_to_dek_ext): Do not deref a NULL PW. + * revoke.c (gen_revoke): Remove unused malloc of PK. + * parse-packet.c (mpi_read): Init NREAD. + Reported by Fabian Keil. + 2009-06-17 Werner Koch * parse-packet.c (parse): Use a casted -1 instead of a 32 bit diff --git a/g10/call-agent.c b/g10/call-agent.c index cd58b90b3..0590514df 100644 --- a/g10/call-agent.c +++ b/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 a/g10/card-util.c b/g10/card-util.c index 9295a1724..26349d653 100644 --- a/g10/card-util.c +++ b/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: diff --git a/g10/keyedit.c b/g10/keyedit.c index b0d59f66a..69429b5ce 100644 --- a/g10/keyedit.c +++ b/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; } @@ -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; diff --git a/g10/keyring.c b/g10/keyring.c index 2c894312d..6b3c48987 100644 --- a/g10/keyring.c +++ b/g10/keyring.c @@ -997,7 +997,7 @@ 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; + /* name = hd->word_match.pattern; */ } init_packet(&pkt); diff --git a/g10/parse-packet.c b/g10/parse-packet.c index a86e54981..16ca7514f 100644 --- a/g10/parse-packet.c +++ b/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; diff --git a/g10/passphrase.c b/g10/passphrase.c index 3742738e9..d34f5fa92 100644 --- a/g10/passphrase.c +++ b/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); diff --git a/g10/revoke.c b/g10/revoke.c index cc66dfced..cce6d69f6 100644 --- a/g10/revoke.c +++ b/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); diff --git a/kbx/keybox-search.c b/kbx/keybox-search.c index 08b59e649..1680dd732 100644 --- a/kbx/keybox-search.c +++ b/kbx/keybox-search.c @@ -739,6 +739,8 @@ keybox_search (KEYBOX_HANDLE hd, KEYBOX_SEARCH_DESC *desc, size_t ndesc) } } + (void)need_words; /* Not yet implemented. */ + if (!hd->fp) { hd->fp = fopen (hd->kb->fname, "rb"); diff --git a/keyserver/ChangeLog b/keyserver/ChangeLog index 2b69ccd56..d09152e46 100644 --- a/keyserver/ChangeLog +++ b/keyserver/ChangeLog @@ -1,3 +1,8 @@ +2009-06-24 Werner Koch + + * gpgkeys_ldap.c (send_key): Do not deep free a NULL modlist. + Reported by Fabian Keil. + 2009-05-28 David Shaw From 1.4: diff --git a/keyserver/gpgkeys_ldap.c b/keyserver/gpgkeys_ldap.c index 3fed8c590..bd8523466 100644 --- a/keyserver/gpgkeys_ldap.c +++ b/keyserver/gpgkeys_ldap.c @@ -771,14 +771,16 @@ send_key(int *r_eof) ret=KEYSERVER_OK; fail: - /* Unwind and free the whole modlist structure */ - for(ml=modlist;*ml;ml++) + if (modlist) { - free_mod_values(*ml); - free(*ml); + /* Unwind and free the whole modlist structure */ + for(ml=modlist;*ml;ml++) + { + free_mod_values(*ml); + free(*ml); + } + free(modlist); } - - free(modlist); free(addlist); free(dn); free(key); diff --git a/sm/ChangeLog b/sm/ChangeLog index c59ef06ea..93a9af122 100644 --- a/sm/ChangeLog +++ b/sm/ChangeLog @@ -1,3 +1,10 @@ +2009-06-24 Werner Koch + + * call-dirmngr.c (pattern_from_strlist): Remove dead assignment of N. + * sign.c (gpgsm_sign): Remove dead assignment. + * certreqgen.c (create_request): Assign GPG_ERR_BUG to RC. + Reported by Fabian Keil. + 2009-05-27 Werner Koch * encrypt.c (encrypt_dek): Make use of make_canon_sexp. diff --git a/sm/call-dirmngr.c b/sm/call-dirmngr.c index 914fdd03e..33aebdf13 100644 --- a/sm/call-dirmngr.c +++ b/sm/call-dirmngr.c @@ -747,7 +747,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++) { diff --git a/sm/certreqgen.c b/sm/certreqgen.c index ca791aab8..59e667981 100644 --- a/sm/certreqgen.c +++ b/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); diff --git a/sm/sign.c b/sm/sign.c index 446cd3792..0569052ed 100644 --- a/sm/sign.c +++ b/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) {