From cfdb67df5d5daa13ce785a2f4e87d8f12b673d71 Mon Sep 17 00:00:00 2001 From: Thomas Roessler Date: Mon, 8 Jan 2001 23:09:30 +0000 Subject: [PATCH] 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. --- charset.c | 6 +++--- charset.h | 2 +- check_sec.sh | 24 +++++++++++++++++++----- dotlock.c | 2 +- enter.c | 2 +- gnupgparse.c | 7 +++---- imap/auth_sasl.c | 4 ++-- imap/utf7.c | 24 +++++++++++------------- imap/util.c | 4 ++-- keymap.c | 2 +- lib.c | 18 +++++++++++------- mutt_sasl.c | 2 +- parse.c | 8 +------- pgp.c | 4 ++-- pgppubring.c | 2 +- pop_auth.c | 4 ++-- regex.c | 28 ++++++++++++++-------------- rfc2047.c | 45 ++++++++++++++++++++++++++++----------------- rfc2231.c | 13 +++++++++---- rfc822.c | 2 +- sendlib.c | 29 ++++++++++++----------------- strdup.c | 4 ++-- 22 files changed, 128 insertions(+), 108 deletions(-) diff --git a/charset.c b/charset.c index e7146f3e..6d36e79e 100644 --- a/charset.c +++ b/charset.c @@ -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); } diff --git a/charset.h b/charset.h index b15ce14c..2d281008 100644 --- a/charset.h +++ b/charset.h @@ -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); diff --git a/check_sec.sh b/check_sec.sh index 185752ac..988bb163 100755 --- a/check_sec.sh +++ b/check_sec.sh @@ -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 '\wbuf + state->curpos, savebuf, savelen * sizeof (wchar_t)); state->lastchar = state->curpos + savelen; - free (savebuf); + safe_free ((void **) &savebuf); } /* diff --git a/gnupgparse.c b/gnupgparse.c index 7be19517..c7feca89 100644 --- a/gnupgparse.c +++ b/gnupgparse.c @@ -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) diff --git a/imap/auth_sasl.c b/imap/auth_sasl.c index d1bc0d20..b037f689 100644 --- a/imap/auth_sasl.c +++ b/imap/auth_sasl.c @@ -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) diff --git a/imap/utf7.c b/imap/utf7.c index 4733b814..93031503 100644 --- a/imap/utf7.c +++ b/imap/utf7.c @@ -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; } } diff --git a/imap/util.c b/imap/util.c index 8e15d043..778d39b2 100644 --- a/imap/util.c +++ b/imap/util.c @@ -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 */ diff --git a/keymap.c b/keymap.c index dae5fea2..0b97906d 100644 --- a/keymap.c +++ b/keymap.c @@ -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)) diff --git a/lib.c b/lib.c index 27f4f823..9db9ffd8 100644 --- a/lib.c +++ b/lib.c @@ -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; diff --git a/mutt_sasl.c b/mutt_sasl.c index f896c4fa..6714463e 100644 --- a/mutt_sasl.c +++ b/mutt_sasl.c @@ -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__ */ diff --git a/parse.c b/parse.c index 95d62fa1..7ce2e83e 100644 --- a/parse.c +++ b/parse.c @@ -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); } diff --git a/pgp.c b/pgp.c index 1caf98e9..dc5debdd 100644 --- a/pgp.c +++ b/pgp.c @@ -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) { diff --git a/pgppubring.c b/pgppubring.c index aa5d5b79..aa05822a 100644 --- a/pgppubring.c +++ b/pgppubring.c @@ -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; diff --git a/pop_auth.c b/pop_auth.c index 2dc3bb1a..a4f6bbd7 100644 --- a/pop_auth.c +++ b/pop_auth.c @@ -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); } } diff --git a/regex.c b/regex.c index 36bf7b3d..398f7f9d 100644 --- a/regex.c +++ b/regex.c @@ -109,8 +109,8 @@ #if defined (STDC_HEADERS) || defined (_LIBC) #include #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; } diff --git a/rfc2047.c b/rfc2047.c index 42fe87fb..a28c8db4 100644 --- a/rfc2047.c +++ b/rfc2047.c @@ -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; } diff --git a/rfc2231.c b/rfc2231.c index f98318ad..fe22c218 100644 --- a/rfc2231.c +++ b/rfc2231.c @@ -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; } + diff --git a/rfc822.c b/rfc822.c index d22411bb..27f4d38c 100644 --- a/rfc822.c +++ b/rfc822.c @@ -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; } diff --git a/sendlib.c b/sendlib.c index 42ee234f..45ad8aa8 100644 --- a/sendlib.c +++ b/sendlib.c @@ -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; } diff --git a/strdup.c b/strdup.c index ce25a2f7..0249a058 100644 --- a/strdup.c +++ b/strdup.c @@ -3,14 +3,14 @@ #include #include -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); -- 2.40.0