From 503d12ff6d253b25eaf6a2dc4c24ee54a825bbc3 Mon Sep 17 00:00:00 2001 From: Rocco Rutte Date: Thu, 18 May 2006 17:35:29 +0000 Subject: [PATCH] Avoid safe_free() usage and add security checks Add checks to check_sec.sh for memory functions. These include a check for use of safe_free() instead of FREE() and a check whether FREE(&...) is used. For the former, __SAFE_FREE_CHECKED__ is to be used, for the latter __FREE_CHECKED__ to avoid messages from check_sec.sh --- buffy.c | 4 ++-- charset.c | 4 ++-- check_sec.sh | 2 ++ color.c | 2 +- enter.c | 2 +- group.c | 2 +- hash.c | 2 +- imap/message.c | 2 +- imap/utf7.c | 4 ++-- imap/util.c | 2 +- init.c | 24 ++++++++++++++---------- lib.c | 4 ++-- menu.c | 2 +- mh.c | 4 ++-- mutt_idna.c | 4 ++-- mutt_ssl_gnutls.c | 2 +- muttlib.c | 18 +++++++++--------- mx.c | 2 +- pager.c | 2 +- pgplib.c | 3 ++- remailer.c | 4 ++-- rfc1524.c | 2 +- rfc2047.c | 4 ++-- rfc2231.c | 6 +++--- rfc822.c | 2 +- sendlib.c | 10 +++++----- 26 files changed, 63 insertions(+), 56 deletions(-) diff --git a/buffy.c b/buffy.c index f359529a..3c059400 100644 --- a/buffy.c +++ b/buffy.c @@ -188,7 +188,7 @@ int mutt_parse_mailboxes (BUFFER *path, BUFFER *s, unsigned long data, BUFFER *e { FREE (&((*tmp)->path)); tmp1=(*tmp)->next; - FREE (tmp); + FREE (tmp); /* __FREE_CHECKED__ */ *tmp=tmp1; } return 0; @@ -212,7 +212,7 @@ int mutt_parse_mailboxes (BUFFER *path, BUFFER *s, unsigned long data, BUFFER *e { FREE (&((*tmp)->path)); tmp1=(*tmp)->next; - FREE (tmp); + FREE (tmp); /* __FREE_CHECKED__ */ *tmp=tmp1; } continue; diff --git a/charset.c b/charset.c index 30fc69df..22c62ca1 100644 --- a/charset.c +++ b/charset.c @@ -450,7 +450,7 @@ int mutt_convert_string (char **ps, const char *from, const char *to, int flags) *ob = '\0'; - FREE (ps); + FREE (ps); /* __FREE_CHECKED__ */ *ps = buf; mutt_str_adjust (ps); @@ -589,5 +589,5 @@ void fgetconv_close (FGETCONV **_fc) if (fc->cd != (iconv_t)-1) iconv_close (fc->cd); - FREE (_fc); + FREE (_fc); /* __FREE_CHECKED__ */ } diff --git a/check_sec.sh b/check_sec.sh index 4c5047a5..f8a73e5c 100755 --- a/check_sec.sh +++ b/check_sec.sh @@ -35,6 +35,8 @@ do_check '\<(mutt_)?strcpy' __STRCPY_CHECKED__ "Alert: Unchecked strcpy calls." do_check '\rx); mutt_pattern_free(&tmp->color_pattern); FREE (&tmp->pattern); - FREE (l); + FREE (l); /* __FREE_CHECKED__ */ } void ci_start_color (void) diff --git a/enter.c b/enter.c index bd8ba385..ff86aa02 100644 --- a/enter.c +++ b/enter.c @@ -697,7 +697,7 @@ void mutt_free_enter_state (ENTER_STATE **esp) if (!esp) return; FREE (&(*esp)->wbuf); - FREE (esp); + FREE (esp); /* __FREE_CHECKED__ */ } /* diff --git a/group.c b/group.c index 41960bee..142acace 100644 --- a/group.c +++ b/group.c @@ -75,7 +75,7 @@ void mutt_group_context_destroy (group_context_t **ctx) for (; *ctx; *ctx = p) { p = (*ctx)->next; - FREE (ctx); + FREE (ctx); /* __FREE_CHECKED__ */ } } diff --git a/hash.c b/hash.c index cc9ca15a..2c7e6dd7 100644 --- a/hash.c +++ b/hash.c @@ -158,5 +158,5 @@ void hash_destroy (HASH **ptr, void (*destroy) (void *)) } } FREE (&pptr->table); - FREE (ptr); + FREE (ptr); /* __FREE_CHECKED__ */ } diff --git a/imap/message.c b/imap/message.c index a808b01f..54ec2586 100644 --- a/imap/message.c +++ b/imap/message.c @@ -964,7 +964,7 @@ void imap_free_header_data (void** data) /* this should be safe even if the list wasn't used */ mutt_free_list (&(((IMAP_HEADER_DATA*) *data)->keywords)); - FREE (data); + FREE (data); /* __FREE_CHECKED__ */ } /* imap_set_flags: fill out the message header according to the flags from diff --git a/imap/utf7.c b/imap/utf7.c index 55c903e6..97212ad5 100644 --- a/imap/utf7.c +++ b/imap/utf7.c @@ -260,7 +260,7 @@ void imap_utf7_encode (char **s) if (!mutt_convert_string (&t, Charset, "UTF-8", 0)) { char *u7 = utf8_to_utf7 (t, strlen (t), NULL, 0); - FREE (s); + FREE (s); /* __FREE_CHECKED__ */ *s = u7; } FREE (&t); @@ -274,7 +274,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, 0)) { - FREE (s); + FREE (s); /* __FREE_CHECKED__ */ *s = t; } } diff --git a/imap/util.c b/imap/util.c index da42d2f4..5bfc8511 100644 --- a/imap/util.c +++ b/imap/util.c @@ -278,7 +278,7 @@ void imap_free_idata (IMAP_DATA** idata) imap_mboxcache_free (*idata); mutt_buffer_free(&(*idata)->cmdbuf); FREE (&(*idata)->buf); - FREE (idata); + FREE (idata); /* __FREE_CHECKED__ */ } /* diff --git a/init.c b/init.c index bd23c344..4b415249 100644 --- a/init.c +++ b/init.c @@ -346,7 +346,7 @@ static void mutt_free_opt (struct option_t* p) break; case DT_PATH: case DT_STR: - FREE ((char**)p->data); + FREE ((char**)p->data); /* __FREE_CHECKED__ */ break; } } @@ -477,7 +477,7 @@ static int add_to_spam_list (SPAM_LIST **list, const char *pat, const char *temp * the template, and leaving t pointed at the current item. */ t = last; - safe_free(&t->template); + FREE(&t->template); break; } if (!last->next) @@ -533,8 +533,8 @@ static int remove_from_spam_list (SPAM_LIST **list, const char *pat) { *list = spam->next; mutt_free_regexp(&spam->rx); - safe_free(&spam->template); - safe_free(&spam); + FREE(&spam->template); + FREE(&spam); return 1; } @@ -545,8 +545,8 @@ static int remove_from_spam_list (SPAM_LIST **list, const char *pat) { prev->next = spam->next; mutt_free_regexp(&spam->rx); - safe_free(&spam->template); - safe_free(&spam); + FREE(&spam->template); + FREE(&spam); spam = prev->next; ++nremoved; } @@ -981,7 +981,7 @@ static int parse_attach_list (BUFFER *buf, BUFFER *s, LIST **ldata, BUFFER *err) a->major_int = mutt_check_mime_type(a->major); regcomp(&a->minor_rx, tmpminor, REG_ICASE|REG_EXTENDED); - safe_free(&tmpminor); + FREE(&tmpminor); dprint(5, (debugfile, "parse_attach_list: added %s/%s [%d]\n", a->major, a->minor, a->major_int)); @@ -1759,7 +1759,9 @@ static int parse_set (BUFFER *tmp, BUFFER *s, unsigned long data, BUFFER *err) else if (DTYPE (MuttVars[idx].type) == DT_ADDR) rfc822_free_address ((ADDRESS **) MuttVars[idx].data); else - FREE ((void *) MuttVars[idx].data); + /* MuttVars[idx].data is already 'char**' (or some 'void**') or... + * so cast to 'void*' is okay */ + FREE ((void *) MuttVars[idx].data); /* __FREE_CHECKED__ */ } else if (query || *s->dptr != '=') { @@ -1807,7 +1809,9 @@ static int parse_set (BUFFER *tmp, BUFFER *s, unsigned long data, BUFFER *err) else if (DTYPE (MuttVars[idx].type) == DT_ADDR) rfc822_free_address ((ADDRESS **) MuttVars[idx].data); else - FREE ((void *) MuttVars[idx].data); + /* MuttVars[idx].data is already 'char**' (or some 'void**') or... + * so cast to 'void*' is okay */ + FREE ((void *) MuttVars[idx].data); /* __FREE_CHECKED__ */ mutt_extract_token (tmp, s, 0); if (myvar) @@ -3080,7 +3084,7 @@ static void myvar_del (const char* var) tmp = (*cur)->next; FREE (&(*cur)->name); FREE (&(*cur)->value); - FREE (cur); + FREE (cur); /* __FREE_CHECKED__ */ *cur = tmp; } } diff --git a/lib.c b/lib.c index 5ac4133c..aac07425 100644 --- a/lib.c +++ b/lib.c @@ -185,7 +185,7 @@ void safe_realloc (void *ptr, size_t siz) *p = r; } -void safe_free (void *ptr) +void safe_free (void *ptr) /* __SAFE_FREE_CHECKED__ */ { void **p = (void **)ptr; if (*p) @@ -260,7 +260,7 @@ char *safe_strncat (char *d, size_t l, const char *s, size_t sl) void mutt_str_replace (char **p, const char *s) { - FREE (p); + FREE (p); /* __FREE_CHECKED__ */ *p = safe_strdup (s); } diff --git a/menu.c b/menu.c index 332bbdfc..cbb3a8db 100644 --- a/menu.c +++ b/menu.c @@ -703,7 +703,7 @@ void mutt_menuDestroy (MUTTMENU **p) FREE (& (*p)->dialog); } - FREE (p); + FREE (p); /* __FREE_CHECKED__ */ } #define M_SEARCH_UP 1 diff --git a/mh.c b/mh.c index 2824dedc..3a4e44cd 100644 --- a/mh.c +++ b/mh.c @@ -222,7 +222,7 @@ static int mh_mkstemp (CONTEXT * dest, FILE ** fp, char **tgt) if ((*fp = fdopen (fd, "w")) == NULL) { - FREE (tgt); + FREE (tgt); /* __FREE_CHECKED__ */ close (fd); unlink (path); return (-1); @@ -488,7 +488,7 @@ static void maildir_free_entry (struct maildir **md) if ((*md)->h) mutt_free_header (&(*md)->h); - FREE (md); + FREE (md); /* __FREE_CHECKED__ */ } static void maildir_free_maildir (struct maildir **md) diff --git a/mutt_idna.c b/mutt_idna.c index 36915a45..52138b3f 100644 --- a/mutt_idna.c +++ b/mutt_idna.c @@ -89,7 +89,7 @@ int mutt_idna_to_local (const char *in, char **out, int flags) return 0; notrans: - FREE (out); + FREE (out); /* __FREE_CHECKED__ */ *out = safe_strdup (in); return 1; } @@ -114,7 +114,7 @@ int mutt_local_to_idna (const char *in, char **out) FREE (&tmp); if (rv < 0) { - FREE (out); + FREE (out); /* __FREE_CHECKED__ */ *out = safe_strdup (in); } return rv; diff --git a/mutt_ssl_gnutls.c b/mutt_ssl_gnutls.c index 1f1bca23..136ef931 100644 --- a/mutt_ssl_gnutls.c +++ b/mutt_ssl_gnutls.c @@ -288,7 +288,7 @@ static int tls_socket_close (CONNECTION* conn) gnutls_certificate_free_credentials (data->xcred); gnutls_deinit (data->state); - safe_free ((void **) &conn->sockdata); + FREE (&conn->sockdata); } return raw_socket_close (conn); diff --git a/muttlib.c b/muttlib.c index 65ffec02..64f9ef7a 100644 --- a/muttlib.c +++ b/muttlib.c @@ -291,7 +291,7 @@ void mutt_free_header (HEADER **h) #if defined USE_POP || defined USE_IMAP FREE (&(*h)->data); #endif - FREE (h); + FREE (h); /* __FREE_CHECKED__ */ } /* returns true if the header contained in "s" is in list "t" */ @@ -679,7 +679,7 @@ void mutt_free_envelope (ENVELOPE **p) mutt_free_list (&(*p)->references); mutt_free_list (&(*p)->in_reply_to); mutt_free_list (&(*p)->userhdrs); - FREE (p); + FREE (p); /* __FREE_CHECKED__ */ } /* move all the headers from extra not present in base into base */ @@ -913,13 +913,13 @@ int mutt_check_overwrite (const char *attname, const char *path, mutt_str_replace (directory, fname); break; case 1: /* yes */ - FREE (directory); + FREE (directory); /* __FREE_CHECKED__ */ break; case -1: /* abort */ - FREE (directory); + FREE (directory); /* __FREE_CHECKED__ */ return -1; case 2: /* no */ - FREE (directory); + FREE (directory); /* __FREE_CHECKED__ */ return 1; } } @@ -1393,7 +1393,7 @@ BUFFER * mutt_buffer_init(BUFFER *b) } else { - safe_free(&b->data); + FREE(&b->data); } memset(b, 0, sizeof(BUFFER)); return b; @@ -1473,7 +1473,7 @@ void mutt_buffer_free (BUFFER **p) FREE(&(*p)->data); /* dptr is just an offset to data and shouldn't be freed */ - FREE(p); + FREE(p); /* __FREE_CHECKED__ */ } /* dynamically grows a BUFFER to accomodate s, in increments of 128 bytes. @@ -1545,7 +1545,7 @@ void mutt_free_regexp (REGEXP **pp) FREE (&(*pp)->pattern); regfree ((*pp)->rx); FREE (&(*pp)->rx); - FREE (pp); + FREE (pp); /* __FREE_CHECKED__ */ } void mutt_free_rx_list (RX_LIST **list) @@ -1572,7 +1572,7 @@ void mutt_free_spam_list (SPAM_LIST **list) p = *list; *list = (*list)->next; mutt_free_regexp (&p->rx); - safe_free(&p->template); + FREE (&p->template); FREE (&p); } } diff --git a/mx.c b/mx.c index 1c7167f4..e12f875b 100644 --- a/mx.c +++ b/mx.c @@ -1526,7 +1526,7 @@ int mx_close_message (MESSAGE **msg) FREE (&(*msg)->path); } - FREE (msg); + FREE (msg); /* __FREE_CHECKED__ */ return (r); } diff --git a/pager.c b/pager.c index 440d7c6d..9231877b 100644 --- a/pager.c +++ b/pager.c @@ -389,7 +389,7 @@ cleanup_quote (struct q_class_t **QuoteList) ptr = (*QuoteList)->next; if ((*QuoteList)->prefix) FREE (&(*QuoteList)->prefix); - FREE (QuoteList); + FREE (QuoteList); /* __FREE_CHECKED__ */ *QuoteList = ptr; } diff --git a/pgplib.c b/pgplib.c index 2456058a..85db5631 100644 --- a/pgplib.c +++ b/pgplib.c @@ -183,7 +183,8 @@ static void _pgp_free_key (pgp_key_t *kpp) pgp_free_uid (&kp->address); FREE (&kp->keyid); - FREE (kpp); + /* mutt_crypt.h: 'typedef struct pgp_keyinfo *pgp_key_t;' */ + FREE (kpp); /* __FREE_CHECKED__ */ } pgp_key_t pgp_remove_key (pgp_key_t *klist, pgp_key_t key) diff --git a/remailer.c b/remailer.c index 8e2d8116..78680fd2 100644 --- a/remailer.c +++ b/remailer.c @@ -126,7 +126,7 @@ static void mix_free_remailer (REMAILER **r) FREE (&(*r)->addr); FREE (&(*r)->ver); - FREE (r); + FREE (r); /* __FREE_CHECKED__ */ } /* parse the type2.list as given by mixmaster -T */ @@ -216,7 +216,7 @@ static void mix_free_type2_list (REMAILER ***ttlp) for (i = 0; type2_list[i]; i++) mix_free_remailer (&type2_list[i]); - FREE (type2_list); + FREE (type2_list); /* __FREE_CHECKED__ */ } diff --git a/rfc1524.c b/rfc1524.c index c6ec3126..6fe0ffa1 100644 --- a/rfc1524.c +++ b/rfc1524.c @@ -368,7 +368,7 @@ void rfc1524_free_entry(rfc1524_entry **entry) FREE (&p->editcommand); FREE (&p->printcommand); FREE (&p->nametemplate); - FREE (entry); + FREE (entry); /* __FREE_CHECKED__ */ } /* diff --git a/rfc2047.c b/rfc2047.c index a2159a02..158423e6 100644 --- a/rfc2047.c +++ b/rfc2047.c @@ -564,7 +564,7 @@ void _rfc2047_encode_string (char **pd, int encode_specials, int col) Charset, charsets, &e, &elen, encode_specials ? RFC822Specials : NULL); - FREE (pd); + FREE (pd); /* __FREE_CHECKED__ */ *pd = e; } @@ -761,7 +761,7 @@ void rfc2047_decode (char **pd) } *d = 0; - FREE (pd); + FREE (pd); /* __FREE_CHECKED__ */ *pd = d0; mutt_str_adjust (pd); } diff --git a/rfc2231.c b/rfc2231.c index 8cf0289e..3ad3b3ab 100644 --- a/rfc2231.c +++ b/rfc2231.c @@ -183,7 +183,7 @@ static void rfc2231_free_parameter (struct rfc2231_parameter **p) { FREE (&(*p)->attribute); FREE (&(*p)->value); - FREE (p); + FREE (p); /* __FREE_CHECKED__ */ } } @@ -364,12 +364,12 @@ int rfc2231_encode_string (char **pd) if (d != *pd) FREE (&d); - FREE (pd); + FREE (pd); /* __FREE_CHECKED__ */ *pd = e; } else if (d != *pd) { - FREE (pd); + FREE (pd); /* __FREE_CHECKED__ */ *pd = d; } diff --git a/rfc822.c b/rfc822.c index 4ca157fe..a1796b0a 100644 --- a/rfc822.c +++ b/rfc822.c @@ -792,7 +792,7 @@ ADDRESS *rfc822_append (ADDRESS **a, ADDRESS *b) } #ifdef TESTING -int safe_free (void **p) +int safe_free (void **p) /* __SAFE_FREE_CHECKED__ */ { free(*p); /* __MEM_CHECKED__ */ *p = 0; diff --git a/sendlib.c b/sendlib.c index 71602da1..124af627 100644 --- a/sendlib.c +++ b/sendlib.c @@ -828,7 +828,7 @@ static size_t convert_file_from_to (FILE *file, for (i = 0; i < ncodes; i++) FREE (&tcode[i]); - FREE (tcode); + FREE (tcode); /* __FREE_CHECKED__ */ return ret; } @@ -1819,7 +1819,7 @@ send_msg (const char *path, char **args, const char *msg, char **tempfile) else if (pid == -1) { unlink (msg); - FREE (tempfile); + FREE (tempfile); /* __FREE_CHECKED__ */ _exit (S_ERR); } @@ -1850,7 +1850,7 @@ send_msg (const char *path, char **args, const char *msg, char **tempfile) if (SendmailWait && st == (0xff & EX_OK)) { unlink (*tempfile); /* no longer needed */ - FREE (tempfile); + FREE (tempfile); /* __FREE_CHECKED__ */ } } else @@ -1860,7 +1860,7 @@ send_msg (const char *path, char **args, const char *msg, char **tempfile) if (SendmailWait > 0) { unlink (*tempfile); - FREE (tempfile); + FREE (tempfile); /* __FREE_CHECKED__ */ } } @@ -1872,7 +1872,7 @@ send_msg (const char *path, char **args, const char *msg, char **tempfile) { /* the parent is already dead */ unlink (*tempfile); - FREE (tempfile); + FREE (tempfile); /* __FREE_CHECKED__ */ } _exit (st); -- 2.40.0