On 2004-10-30 02:26:31 +0200, Ulf Härnhammar wrote:
> All problems were found in the latest CVS, although some of them
> exist in the stable version 1.4.2.1 as well. I don't _think_ any
> of these problems pose any big security threat, but it is good to
> fix these things anyway to avoid crashes. I have attached a patch
> against the latest CVS version.
Thanks for noting these. I'll go through them in steps...
> --- smime.c.old 2004-08-08 18:19:15.000000000 +0200
> +++ smime.c 2004-10-29 17:02:59.000000000 +0200
> @@ -557,7 +557,7 @@ char *smime_get_field_from_db (char *mai
> }
> else if (choice == M_YES)
> {
> - snprintf (key,mutt_strlen(key)+1, fields[1]);
> + snprintf (key, mutt_strlen(key)+1, "%s", fields[1]);
> ask = 0;
> break;
> }
> @@ -573,8 +573,12 @@ char *smime_get_field_from_db (char *mai
> }
> else if(query)
> {
> - numFields = sscanf (buf, "%s %s %s %s %s\n", fields[0], fields[1],
> - fields[2], fields[3], fields[4]);
> + numFields = sscanf (buf,
> + MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) " "
> + MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) " "
> + MUTT_FORMAT(STRING) "\n",
> + fields[0], fields[1], fields[2], fields[3],
> + fields[4]);
>
> /* query = label: return certificate. */
> if (numFields >= 3 &&
I'm committing the attached patch instead.
Basically, smime_get_field_from_db was doing quite a bit of
reallocating and other stuff just to append a newline to the string
stored in key, but that newline was only needed in a single place
(where it could be added much easier), and the length of key was
naturally limited by the length of a single field.
(Note that I don't have S/MIME installed here, so this is untested.
Please report back whether the change breaks anything.)
--
Thomas Roessler · Personal soap box at <http://log.does-not-exist.org/>.
Index: smime.c
===================================================================
RCS file: /cvs/mutt/mutt/smime.c,v
retrieving revision 3.34
diff -u -p -r3.34 smime.c
--- smime.c 8 Aug 2004 16:19:15 -0000 3.34
+++ smime.c 30 Oct 2004 20:44:20 -0000
@@ -479,21 +479,22 @@ char *smime_get_field_from_db (char *mai
char cert_path[_POSIX_PATH_MAX];
char buf[LONG_STRING], prompt[STRING];
char fields[5][STRING];
+ char key[STRING];
int numFields;
struct stat info;
- char *key=NULL, key_trust_level = 0;
+ char key_trust_level = 0;
FILE *fp;
if(!mailbox && !query) return(NULL);
addr_len = mailbox ? mutt_strlen (mailbox) : 0;
query_len = query ? mutt_strlen (query) : 0;
+
+ *key = '\0';
/* index-file format:
mailbox certfile label issuer_certfile trust_flags\n
- \n is also copied here, serving as delimitation.
-
certfile is a hash value generated by openssl.
Note that this was done according to the OpenSSL
specs on their CA-directory.
@@ -546,8 +547,7 @@ char *smime_get_field_from_db (char *mai
{
found = 0;
ask = 0;
- FREE (&key);
- key = NULL;
+ *key = '\0';
break;
}
else if (choice == M_NO)
@@ -557,44 +557,46 @@ char *smime_get_field_from_db (char *mai
}
else if (choice == M_YES)
{
- snprintf (key,mutt_strlen(key)+1, fields[1]);
+ strfcpy (key, fields[1], sizeof (key));
ask = 0;
break;
}
}
else
{
- key = safe_calloc(1, mutt_strlen(fields[1])+2);
- if (public) key_trust_level = *fields[4];
- snprintf(key, mutt_strlen(fields[1])+1, "%s", fields[1]);
-
+ if (public)
+ key_trust_level = *fields[4];
+ strfcpy (key, fields[1], sizeof (key));
}
found = 1;
}
else if(query)
{
- numFields = sscanf (buf, "%s %s %s %s %s\n", fields[0], fields[1],
- fields[2], fields[3], fields[4]);
+ numFields = sscanf (buf,
+ MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) " "
+ MUTT_FORMAT(STRING) " " MUTT_FORMAT(STRING) " "
+ MUTT_FORMAT(STRING) "\n",
+ fields[0], fields[1],
+ fields[2], fields[3],
+ fields[4]);
/* query = label: return certificate. */
if (numFields >= 3 &&
!(mutt_strncasecmp (query, fields[2], query_len)))
{
ask = 0;
- key = safe_calloc(1, mutt_strlen(fields[1])+2);
- snprintf(key, mutt_strlen(fields[1])+1, "%s", fields[1]);
+ strfcpy (key, fields[1], sizeof (key));
}
/* query = certificate: return intermediate certificate. */
else if (numFields >= 4 &&
!(mutt_strncasecmp (query, fields[1], query_len)))
{
ask = 0;
- key = safe_calloc(1, mutt_strlen(fields[3])+2);
- snprintf(key, mutt_strlen(fields[3])+1, "%s", fields[3]);
+ strfcpy (key, fields[3], sizeof (key));
}
}
- fclose (fp);
+ safe_fclose (&fp);
if (ask)
{
@@ -611,10 +613,7 @@ char *smime_get_field_from_db (char *mai
mailbox);
choice = mutt_yesorno (prompt, M_NO);
if (choice == -1 || choice == M_NO)
- {
- FREE (&key);
- key = NULL;
- }
+ *key = '\0';
}
else if (key_trust_level && may_ask)
{
@@ -625,11 +624,7 @@ char *smime_get_field_from_db (char *mai
key, mailbox);
choice = mutt_yesorno (prompt, M_NO);
if (choice != M_YES)
- {
- FREE (&key);
- key = NULL;
- }
-
+ *key = '\0';
}
else if (key_trust_level == 'v' )
{
@@ -640,13 +635,8 @@ char *smime_get_field_from_db (char *mai
}
- if (key)
- {
- key[mutt_strlen(key)+1] = '\0';
- key[mutt_strlen(key)] = '\n';
- }
-
- return key;
+ /* Note: safe_strdup ("") returns NULL. */
+ return safe_strdup (key);
}
@@ -673,8 +663,6 @@ void _smime_getkeys (char *mailbox)
if (k)
{
- k[mutt_strlen (k)-1] = '\0';
-
/* the key used last time. */
if (*SmimeKeyToUse &&
!mutt_strcasecmp (k, SmimeKeyToUse + mutt_strlen (SmimeKeys)+1))
@@ -806,9 +794,9 @@ char *smime_findKeys (ADDRESS *to, ADDRE
return NULL;
}
- keylist_size += mutt_strlen (keyID) + 1;
+ keylist_size += mutt_strlen (keyID) + 2;
safe_realloc (&keylist, keylist_size);
- sprintf (keylist + keylist_used, "%s", keyID); /* __SPRINTF_CHECKED__
*/
+ sprintf (keylist + keylist_used, "%s\n", keyID); /* __SPRINTF_CHECKED__
*/
keylist_used = mutt_strlen (keylist);
rfc822_free_address (&addr);
@@ -1396,8 +1384,6 @@ BODY *smime_sign_message (BODY *a )
mutt_message(_("Warning: Intermediate certificate not found."));
intermediates = SmimeDefaultKey; /* so openssl won't complain in any case
*/
}
- else
- intermediates[mutt_strlen (intermediates)-1] = '\0';
convert_to_7bit (a); /* Signed data _must_ be in 7-bit format. */
Attachment:
pgplTvXjQcK7I.pgp
Description: PGP signature