Commit cfdb67df authored by Thomas Roessler's avatar Thomas Roessler

Let check_sec.sh check for use of the unsafe malloc, realloc, free,

and strdup routines.  While we are on it, plug some memory leaks and
make some code understandable.
parent d5a33624
......@@ -514,11 +514,11 @@ int fgetconv (FGETCONV *_fc)
return EOF;
}
void fgetconv_close (FGETCONV *_fc)
void fgetconv_close (FGETCONV **_fc)
{
struct fgetconv_s *fc = (struct fgetconv_s *)_fc;
struct fgetconv_s *fc = (struct fgetconv_s *) *_fc;
if (fc->cd != (iconv_t)-1)
iconv_close (fc->cd);
free (fc);
safe_free (_fc);
}
......@@ -30,7 +30,7 @@ typedef void * FGETCONV;
FGETCONV *fgetconv_open (FILE *, const char *, const char *);
int fgetconv (FGETCONV *);
void fgetconv_close (FGETCONV *);
void fgetconv_close (FGETCONV **);
void mutt_set_langinfo_charset (void);
......
......@@ -6,22 +6,36 @@
TMPFILE="`mktemp check_sec.tmp.XXXXXX`" || exit 1
do_check ()
do_check_files ()
{
egrep -n "$1" *.c */*.c | fgrep -v $2 > $TMPFILE
pattern="$1" ; shift
magic="$1" ; shift
msg="$1" ; shift
egrep -n "$pattern" "$@" | \
grep -v '^[^ ]*:[^ ]*#' | \
fgrep -v "$magic" > $TMPFILE
test -s $TMPFILE && {
echo "$3" ;
echo "$msg" ;
cat $TMPFILE;
rm -f $TMPFILE;
exit 1;
}
}
do_check ()
{
do_check_files "$1" "$2" "$3" *.c */*.c
}
do_check '\<fopen.*'\"'.*w' __FOPEN_CHECKED__ "Alert: Unchecked fopen calls."
do_check '\<(mutt_)?strcpy' __STRCPY_CHECKED__ "Alert: Unchecked strcpy calls."
do_check '\<strcat' __STRCAT_CHECKED__ "Alert: Unchecked strcat calls."
do_check 'sprintf.*%s' __SPRINTF_CHECKED__ "Alert: Unchecked sprintf calls."
do_check '\<sprintf.*%s' __SPRINTF_CHECKED__ "Alert: Unchecked sprintf calls."
# don't do this check on others' code.
do_check_files '\<(malloc|realloc|free|strdup)[ ]*\(' __MEM_CHECKED__ "Alert: Use of traditional memory management calls." \
*.c imap/*.c
rm -f $TMPFILE
exit 0
......@@ -146,7 +146,7 @@ int main (int argc, char **argv)
/* determine the system's host name */
uname (&utsname);
if (!(Hostname = strdup (utsname.nodename)))
if (!(Hostname = strdup (utsname.nodename))) /* __MEM_CHECKED__ */
return DL_EX_ERROR;
if ((p = strchr (Hostname, '.')))
*p = '\0';
......
......@@ -166,7 +166,7 @@ static void replace_part (ENTER_STATE *state, size_t from, char *buf)
memcpy (state->wbuf + state->curpos, savebuf, savelen * sizeof (wchar_t));
state->lastchar = state->curpos + savelen;
free (savebuf);
safe_free ((void **) &savebuf);
}
/*
......
......@@ -108,7 +108,7 @@ static void fix_uid (char *uid)
else if (ob-buf == n && (buf[n] = 0, strlen (buf) < n))
memcpy (uid, buf, n);
}
free (buf);
safe_free ((void **) &buf);
iconv_close (cd);
}
}
......@@ -291,9 +291,8 @@ pgp_key_t *pgp_get_candidates (pgp_ring_t keyring, LIST * hints)
if ((devnull = open ("/dev/null", O_RDWR)) == -1)
return NULL;
free (_chs);
_chs = safe_strdup (Charset);
mutt_str_replace (&_chs, Charset);
thepid = pgp_invoke_list_keys (NULL, &fp, NULL, -1, -1, devnull,
keyring, hints);
if (thepid == -1)
......
......@@ -37,7 +37,7 @@ imap_auth_res_t imap_auth_sasl (IMAP_DATA* idata)
int rc, irc;
char buf[LONG_STRING];
const char* mech;
char* pc;
char* pc = NULL;
unsigned int len, olen;
unsigned char client_start;
......@@ -163,7 +163,7 @@ imap_auth_res_t imap_auth_sasl (IMAP_DATA* idata)
/* sasl_client_st(art|ep) allocate pc with malloc, expect me to
* free it */
free (pc);
safe_free (&pc);
}
if (olen || rc == SASL_CONTINUE)
......
......@@ -123,14 +123,14 @@ static char *utf7_to_utf8 (const char *u7, size_t u7len, char **u8,
*p++ = '\0';
if (u8len)
*u8len = p - buf;
p = realloc (buf, p - buf);
p = p ? p : buf;
safe_realloc ((void **) &buf, p - buf);
if (u8)
*u8 = p;
return p;
*u8 = buf;
return buf;
bail:
free (buf);
safe_free ((void **) &buf);
return 0;
}
......@@ -220,7 +220,7 @@ static char *utf8_to_utf7 (const char *u8, size_t u8len, char **u7,
if (u8len)
{
free (buf);
safe_free ((void **) &buf);
return 0;
}
......@@ -234,14 +234,12 @@ static char *utf8_to_utf7 (const char *u8, size_t u8len, char **u7,
*p++ = '\0';
if (u7len)
*u7len = p - buf;
p = realloc (buf, p - buf);
p = p ? p : buf;
if (u7)
*u7 = p;
return p;
safe_realloc ((void **) &buf, p - buf);
if (u7) *u7 = buf;
return buf;
bail:
free (buf);
safe_free ((void **) &buf);
return 0;
}
......@@ -263,7 +261,7 @@ void imap_utf7_decode (char **s)
char *t = utf7_to_utf8 (*s, strlen (*s), 0, 0);
if (t && !mutt_convert_string (&t, "UTF-8", Charset))
{
free (*s);
safe_free ((void **) s);
*s = t;
}
}
......
......@@ -308,7 +308,7 @@ void imap_munge_mbox_name (char *dest, size_t dlen, const char *src)
imap_quote_string (dest, dlen, buf);
free (buf);
safe_free ((void **) &buf);
}
void imap_unmunge_mbox_name (char *s)
......@@ -324,7 +324,7 @@ void imap_unmunge_mbox_name (char *s)
strncpy (s, buf, strlen (s));
}
free (buf);
safe_free ((void **) &buf);
}
/* imap_wordcasecmp: find word a in word list b */
......
......@@ -812,7 +812,7 @@ int mutt_parse_macro (BUFFER *buf, BUFFER *s, unsigned long data, BUFFER *err)
{
if (MoreArgs (s))
{
seq = strdup (buf->data);
seq = safe_strdup (buf->data);
mutt_extract_token (buf, s, M_TOKEN_CONDENSE);
if (MoreArgs (s))
......
......@@ -69,7 +69,7 @@ void *safe_malloc (size_t siz)
if (siz == 0)
return 0;
if ((p = (void *) malloc (siz)) == 0)
if ((p = (void *) malloc (siz)) == 0) /* __MEM_CHECKED__ */
{
mutt_error _("Out of memory!");
sleep (1);
......@@ -86,18 +86,18 @@ void safe_realloc (void **p, size_t siz)
{
if (*p)
{
free (*p);
free (*p); /* __MEM_CHECKED__ */
*p = NULL;
}
return;
}
if (*p)
r = (void *) realloc (*p, siz);
r = (void *) realloc (*p, siz); /* __MEM_CHECKED__ */
else
{
/* realloc(NULL, nbytes) doesn't seem to work under SunOS 4.1.x */
r = (void *) malloc (siz);
/* realloc(NULL, nbytes) doesn't seem to work under SunOS 4.1.x --- __MEM_CHECKED__ */
r = (void *) malloc (siz); /* __MEM_CHECKED__ */
}
if (!r)
......@@ -114,7 +114,7 @@ void safe_free (void **p)
{
if (*p)
{
free (*p);
free (*p); /* __MEM_CHECKED__ */
*p = 0;
}
}
......@@ -498,7 +498,11 @@ char *mutt_substrdup (const char *begin, const char *end)
size_t len;
char *p;
len = end - begin;
if (end)
len = end - begin;
else
len = strlen (begin);
p = safe_malloc (len + 1);
memcpy (p, begin, len);
p[len] = 0;
......
......@@ -236,7 +236,7 @@ static int mutt_sasl_cb_pass (sasl_conn_t* conn, void* context, int id,
len = strlen (account->pass);
*psecret = (sasl_secret_t*) malloc (sizeof (sasl_secret_t) + len);
*psecret = (sasl_secret_t*) safe_malloc (sizeof (sasl_secret_t) + len);
(*psecret)->len = len;
strcpy ((*psecret)->data, account->pass); /* __STRCPY_CHECKED__ */
......
......@@ -363,13 +363,7 @@ static void parse_content_disposition (char *s, BODY *ct)
s++;
SKIPWS (s);
if ((s = mutt_get_parameter ("filename", (parms = parse_parameters (s)))) != 0)
{
/* free() here because the content-type parsing routine might
* have allocated space if a "name=filename" parameter was
* specified.
*/
mutt_str_replace (&ct->filename, s);
}
if ((s = mutt_get_parameter ("name", parms)) != 0)
ct->form_name = safe_strdup (s);
mutt_free_parameter (&parms);
......@@ -1016,7 +1010,7 @@ int mutt_parse_rfc822_line (ENVELOPE *e, HEADER *hdr, char *line, char *p, short
{
if (hdr && in_reply_to)
{
*in_reply_to = strdup (p);
mutt_str_replace (in_reply_to, p);
if (do_2047)
rfc2047_decode (in_reply_to);
}
......
......@@ -1273,8 +1273,8 @@ char *pgp_findKeys (ADDRESS *to, ADDRESS *cc, ADDRESS *bcc)
return (keylist);
}
/* Warning: "a" is no longer free()d in this routine, you need
* to free() it later. This is necessary for $fcc_attach. */
/* Warning: "a" is no longer freed in this routine, you need
* to free it later. This is necessary for $fcc_attach. */
static BODY *pgp_encrypt_message (BODY *a, char *keylist, int sign)
{
......
......@@ -204,7 +204,7 @@ static int read_material (size_t material, size_t * used, FILE * fp)
nplen = *used + material + CHUNKSIZE;
if (!(p = realloc (pbuf, nplen)))
if (!(p = realloc (pbuf, nplen))) /* __MEM_CHECKED__ */
{
perror ("realloc");
return -1;
......
......@@ -41,7 +41,7 @@ static pop_auth_res_t pop_auth_sasl (POP_DATA *pop_data)
char buf[LONG_STRING];
char inbuf[LONG_STRING];
const char* mech;
char* pc;
char* pc = NULL;
unsigned int len, olen;
unsigned char client_start;
......@@ -130,7 +130,7 @@ static pop_auth_res_t pop_auth_sasl (POP_DATA *pop_data)
/* sasl_client_st(art|ep) allocate pc with malloc, expect me to
* free it */
free (pc);
safe_free ((void) &pc);
}
}
......
......@@ -109,8 +109,8 @@
#if defined (STDC_HEADERS) || defined (_LIBC)
#include <stdlib.h>
#else
char *malloc ();
char *realloc ();
char *malloc (); /* __MEM_CHECKED__ */
char *realloc (); /* __MEM_CHECKED__ */
#endif
/* When used in Emacs's lib-src, we need to get bzero and bcopy somehow.
......@@ -1804,7 +1804,7 @@ static boolean group_in_compile_stack _RE_ARGS ((compile_stack_type
/* Return, freeing storage we allocated. */
#define FREE_STACK_RETURN(value) \
return (free (compile_stack.stack), value)
return (free (compile_stack.stack), value) /* __MEM_CHECKED__ */
static reg_errcode_t
regex_compile (pattern, size, syntax, bufp)
......@@ -2845,7 +2845,7 @@ regex_compile (pattern, size, syntax, bufp)
if (syntax & RE_NO_POSIX_BACKTRACKING)
BUF_PUSH (succeed);
free (compile_stack.stack);
free (compile_stack.stack); /* __MEM_CHECKED__ */
/* We have succeeded; set the length of the buffer. */
bufp->used = b - bufp->buffer;
......@@ -2885,11 +2885,11 @@ regex_compile (pattern, size, syntax, bufp)
#else /* not emacs */
if (! fail_stack.stack)
fail_stack.stack
= (fail_stack_elt_t *) malloc (fail_stack.size
= (fail_stack_elt_t *) malloc (fail_stack.size /* __MEM_CHECKED__ */
* sizeof (fail_stack_elt_t));
else
fail_stack.stack
= (fail_stack_elt_t *) realloc (fail_stack.stack,
= (fail_stack_elt_t *) realloc (fail_stack.stack, /* __MEM_CHECKED__ */
(fail_stack.size
* sizeof (fail_stack_elt_t)));
#endif /* not emacs */
......@@ -5469,12 +5469,12 @@ re_comp (s)
if (!re_comp_buf.buffer)
{
re_comp_buf.buffer = (unsigned char *) malloc (200);
re_comp_buf.buffer = (unsigned char *) malloc (200); /* __MEM_CHECKED__ */
if (re_comp_buf.buffer == NULL)
return gettext (re_error_msgid[(int) REG_ESPACE]);
re_comp_buf.allocated = 200;
re_comp_buf.fastmap = (char *) malloc (1 << BYTEWIDTH);
re_comp_buf.fastmap = (char *) malloc (1 << BYTEWIDTH); /* __MEM_CHECKED__ */
if (re_comp_buf.fastmap == NULL)
return gettext (re_error_msgid[(int) REG_ESPACE]);
}
......@@ -5574,7 +5574,7 @@ regcomp (preg, pattern, cflags)
unsigned i;
preg->translate
= (RE_TRANSLATE_TYPE) malloc (CHAR_SET_SIZE
= (RE_TRANSLATE_TYPE) malloc (CHAR_SET_SIZE /* __MEM_CHECKED__ */
* sizeof (*(RE_TRANSLATE_TYPE)0));
if (preg->translate == NULL)
return (int) REG_ESPACE;
......@@ -5678,8 +5678,8 @@ regexec (preg, string, nmatch, pmatch, eflags)
}
/* If we needed the temporary register info, free the space now. */
free (regs.start);
free (regs.end);
free (regs.start); /* __MEM_CHECKED__ */
free (regs.end); /* __MEM_CHECKED__ */
}
/* We want zero return to mean success, unlike `re_search'. */
......@@ -5735,19 +5735,19 @@ regfree (preg)
regex_t *preg;
{
if (preg->buffer != NULL)
free (preg->buffer);
free (preg->buffer); /* __MEM_CHECKED__ */
preg->buffer = NULL;
preg->allocated = 0;
preg->used = 0;
if (preg->fastmap != NULL)
free (preg->fastmap);
free (preg->fastmap); /* __MEM_CHECKED__ */
preg->fastmap = NULL;
preg->fastmap_accurate = 0;
if (preg->translate != NULL)
free (preg->translate);
free (preg->translate); /* __MEM_CHECKED__ */
preg->translate = NULL;
}
......
......@@ -55,7 +55,7 @@ static size_t convert_string (const char *f, size_t flen,
char **t, size_t *tlen)
{
iconv_t cd;
char *buf, *ob, *x;
char *buf, *ob;
size_t obl, n;
int e;
......@@ -68,16 +68,19 @@ static size_t convert_string (const char *f, size_t flen,
if (n == (size_t)(-1) || iconv (cd, 0, 0, &ob, &obl) == (size_t)(-1))
{
e = errno;
free (buf);
safe_free ((void **) &buf);
iconv_close (cd);
errno = e;
return (size_t)(-1);
}
*ob = '\0';
x = realloc (buf, ob - buf + 1);
*t = x ? x : buf;
*tlen = ob - buf;
safe_realloc ((void **) &buf, ob - buf + 1);
*t = buf;
iconv_close (cd);
return n;
}
......@@ -104,26 +107,33 @@ char *mutt_choose_charset (const char *fromcode, const char *charsets,
continue;
t = safe_malloc (n + 1);
memcpy (t, p, n), t[n] = '\0';
memcpy (t, p, n);
t[n] = '\0';
n = convert_string (u, ulen, fromcode, t, &s, &slen);
if (n == (size_t)(-1))
continue;
if (!tocode || n < bestn)
{
bestn = n;
free (tocode), tocode = t;
safe_free ((void **) &tocode);
tocode = t;
if (d)
free (e), e = s;
{
safe_free ((void **) &e);
e = s;
}
else
free (s);
safe_free ((void **) &s);
elen = slen;
if (!bestn)
break;
}
else
{
free (t);
free (s);
safe_free ((void **) &t);
safe_free ((void **) &s);
}
}
if (tocode)
......@@ -509,16 +519,19 @@ static int rfc2047_encode (const char *d, size_t dlen, int col,
/* Add last encoded word and us-ascii suffix to buffer. */
buflen = bufpos + wlen + (u + ulen - t1);
safe_realloc ((void **) &buf, buflen);
safe_realloc ((void **) &buf, buflen + 1);
r = encode_block (buf + bufpos, t, t1 - t, icode, tocode, encoder);
assert (r == wlen);
bufpos += wlen;
memcpy (buf + bufpos, t1, u + ulen - t1);
free (tocode1);
free (u);
safe_free ((void **) &tocode1);
safe_free ((void **) &u);
buf[buflen] = '\0';
*e = buf;
*elen = buflen;
*elen = buflen + 1;
return ret;
}
......@@ -539,9 +552,7 @@ void _rfc2047_encode_string (char **pd, int encode_specials, int col)
Charset, charsets, &e, &elen,
encode_specials ? RFC822Specials : NULL);
safe_realloc ((void **) &e, elen + 1);
e[elen] = '\0';
free (*pd);
safe_free ((void **) pd);
*pd = e;
}
......
......@@ -327,7 +327,8 @@ int rfc2231_encode_string (char **pd)
*pd, strlen (*pd), &d, &dlen)))
{
charset = safe_strdup (Charset ? Charset : "unknown-8bit");
d = *pd, dlen = strlen (d);
d = *pd;
dlen = strlen (d);
}
if (!mutt_is_us_ascii (charset))
......@@ -356,14 +357,18 @@ int rfc2231_encode_string (char **pd)
*t = '\0';
if (d != *pd)
free (d);
free (*pd);
safe_free ((void **) &d);
safe_free ((void **) pd);
*pd = e;
}
else if (d != *pd)
{
free (*pd);
safe_free ((void **) pd);
*pd = d;
}
safe_free ((void **) &charset);
return encode;
}
......@@ -764,7 +764,7 @@ ADDRESS *rfc822_append (ADDRESS **a, ADDRESS *b)
#ifdef TESTING
int safe_free (void **p)
{
free(*p);
free(*p); /* __MEM_CHECKED__ */
*p = 0;
}
......
......@@ -506,7 +506,7 @@ int mutt_write_mime_body (BODY *a, FILE *f)
else
mutt_copy_stream (fpin, f);
fgetconv_close (fc);
fgetconv_close (&fc);
fclose (fpin);
return (ferror (f) ? -1 : 0);
......@@ -810,16 +810,14 @@ static size_t convert_file_from_to (FILE *file,
char *fcode;
char **tcode;
const char *c, *c1;
size_t n, ret;
size_t ret;
int ncodes, i, cn;
/* Count the tocodes */
ncodes = 0;
for (c = tocodes; c; c = c1 ? c1 + 1 : 0)
{
c1 = strchr (c, ':');
n = c1 ? c1 - c : strlen (c);
if (!n)
if ((c1 = strchr (c, ':')) == c)
continue;
++ncodes;
}
......@@ -828,12 +826,9 @@ static size_t convert_file_from_to (FILE *file,
tcode = safe_malloc (ncodes * sizeof (char *));
for (c = tocodes, i = 0; c; c = c1 ? c1 + 1 : 0, i++)
{
c1 = strchr (c, ':');
n = c1 ? c1 - c : strlen (c);
if (!n)
if ((c1 = strchr (c, ':')) == c)
continue;
tcode[i] = malloc (n+1);
memcpy (tcode[i], c, n), tcode[i][n] = '\0';
tcode[i] = mutt_substrdup (c, c1);
}
ret = (size_t)(-1);
......@@ -842,12 +837,10 @@ static size_t convert_file_from_to (FILE *file,
/* Try each fromcode in turn */
for (c = fromcodes; c; c = c1 ? c1 + 1 : 0)
{
c1 = strchr (c, ':');
n = c1 ? c1 - c : strlen (c);
if (!n)
if ((c1 = strchr (c, ':')) == c)
continue;
fcode = malloc (n+1);
memcpy (fcode, c, n), fcode[n] = '\0';
fcode = mutt_substrdup (c, c1);
ret = convert_file_to (file, fcode, ncodes, (const char **)tcode,
&cn, info);
if (ret != (size_t)(-1))
......@@ -857,7 +850,7 @@ static size_t convert_file_from_to (FILE *file,
tcode[cn] = 0;
break;
}
free (fcode);
safe_free ((void **) &fcode);
}
}
else
......@@ -874,8 +867,10 @@ static size_t convert_file_from_to (FILE *file,
/* Free memory */
for (i = 0; i < ncodes; i++)
free (tcode[i]);
safe_free ((void **) &tcode[i]);
safe_free ((void **) tcode);
return ret;
}
......
......@@ -3,14 +3,14 @@
#include <string.h>
#include <stdlib.h>
char *strdup (const char *s)
char *strdup (const char *s) /* __MEM_CHECKED__ */
{
char *d;
if (s == NULL)
return NULL;
if ((d = malloc (strlen (s) + 1)) == NULL)
if ((d = malloc (strlen (s) + 1)) == NULL) /* __MEM_CHECKED__ */
return NULL;
memcpy (d, s, strlen (s) + 1);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment