Changeset 4640:a115f3ac3b62

Timestamp:

2006-05-18 10:35:29 (3 years ago)

Author:

Rocco Rutte <pdmef@…>

Branch:

HEAD

Message:

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

Files:

• buffy.c (modified) (2 diffs)
• charset.c (modified) (2 diffs)
• check_sec.sh (modified) (1 diff)
• color.c (modified) (1 diff)
• enter.c (modified) (1 diff)
• group.c (modified) (1 diff)
• hash.c (modified) (1 diff)
• imap/message.c (modified) (1 diff)
• imap/utf7.c (modified) (2 diffs)
• imap/util.c (modified) (1 diff)
• init.c (modified) (8 diffs)
• lib.c (modified) (2 diffs)
• menu.c (modified) (1 diff)
• mh.c (modified) (2 diffs)
• mutt_idna.c (modified) (2 diffs)
• mutt_ssl_gnutls.c (modified) (1 diff)
• muttlib.c (modified) (7 diffs)
• mx.c (modified) (1 diff)
• pager.c (modified) (1 diff)
• pgplib.c (modified) (1 diff)
• remailer.c (modified) (2 diffs)
• rfc1524.c (modified) (1 diff)
• rfc2047.c (modified) (2 diffs)
• rfc2231.c (modified) (2 diffs)
• rfc822.c (modified) (1 diff)
• sendlib.c (modified) (5 diffs)

buffy.c

@@ -189 +189 @@
         FREE (&((*tmp)->path));
         tmp1=(*tmp)->next;
-        FREE (tmp);
+        FREE (tmp);             /* __FREE_CHECKED__ */
         *tmp=tmp1;
       }
@@ -213 +213 @@
         FREE (&((*tmp)->path));
         tmp1=(*tmp)->next;
-        FREE (tmp);
+        FREE (tmp);             /* __FREE_CHECKED__ */
         *tmp=tmp1;
       }

charset.c

@@ -451 +451 @@
     *ob = '\0';
 
-    FREE (ps);
+    FREE (ps);          /* __FREE_CHECKED__ */
     *ps = buf;
     
@@ -590 +590 @@
   if (fc->cd != (iconv_t)-1)
     iconv_close (fc->cd);
-  FREE (_fc);
-}
+  FREE (_fc);           /* __FREE_CHECKED__ */
+}

check_sec.sh

@@ -36 +36 @@
 do_check '\<sprintf.*%s' __SPRINTF_CHECKED__ "Alert: Unchecked sprintf calls."
 do_check '\<strncat' __STRNCAT_CHECKED__ "You probably meant safe_strcat here."
+do_check '\<safe_free' __SAFE_FREE_CHECKED__ "You probably meant FREE here."
+do_check '\<FREE[ ]?\([^&]' __FREE_CHECKED__ "You probably meant FREE(&...) here."
 
 # don't do this check on others' code.

color.c

@@ -130 +130 @@
   mutt_pattern_free(&tmp->color_pattern);
   FREE (&tmp->pattern);
-  FREE (l);
+  FREE (l);             /* __FREE_CHECKED__ */
 }
 

enter.c

@@ -698 +698 @@
   
   FREE (&(*esp)->wbuf);
-  FREE (esp);
+  FREE (esp);           /* __FREE_CHECKED__ */
 }
 

group.c

@@ -76 +76 @@
   {
     p = (*ctx)->next;
-    FREE (ctx);
+    FREE (ctx);         /* __FREE_CHECKED__ */
   }
 }

hash.c

@@ -159 +159 @@
   }
   FREE (&pptr->table);
-  FREE (ptr);
+  FREE (ptr);           /* __FREE_CHECKED__ */
 }

imap/message.c

@@ -965 +965 @@
   mutt_free_list (&(((IMAP_HEADER_DATA*) *data)->keywords));
 
-  FREE (data);
+  FREE (data);          /* __FREE_CHECKED__ */
 }
 

imap/utf7.c

@@ -261 +261 @@
     {
       char *u7 = utf8_to_utf7 (t, strlen (t), NULL, 0);
-      FREE (s);
+      FREE (s);         /* __FREE_CHECKED__ */
       *s = u7;
     }
@@ -275 +275 @@
     if (t && !mutt_convert_string (&t, "UTF-8", Charset, 0))
     {
-      FREE (s);
+      FREE (s);         /* __FREE_CHECKED__ */
       *s = t;
     }

imap/util.c

@@ -279 +279 @@
   mutt_buffer_free(&(*idata)->cmdbuf);
   FREE (&(*idata)->buf);
-  FREE (idata);
+  FREE (idata);         /* __FREE_CHECKED__ */
 }
 

init.c

@@ -347 +347 @@
   case DT_PATH:
   case DT_STR:
-    FREE ((char**)p->data);
+    FREE ((char**)p->data);             /* __FREE_CHECKED__ */
     break;
   }
@@ -478 +478 @@
        */
       t = last;
-      safe_free(&t->template);
+      FREE(&t->template);
       break;
     }
@@ -534 +534 @@
     *list = spam->next;
     mutt_free_regexp(&spam->rx);
-    safe_free(&spam->template);
-    safe_free(&spam);
+    FREE(&spam->template);
+    FREE(&spam);
     return 1;
   }
@@ -546 +546 @@
       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;
@@ -982 +982 @@
     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",
@@ -1760 +1760 @@
           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 != '=')
@@ -1808 +1810 @@
           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);
@@ -3081 +3085 @@
     FREE (&(*cur)->name);
     FREE (&(*cur)->value);
-    FREE (cur);
+    FREE (cur);         /* __FREE_CHECKED__ */
     *cur = tmp;
   }

lib.c

@@ -186 +186 @@
 }
 
-void safe_free (void *ptr)
+void safe_free (void *ptr)      /* __SAFE_FREE_CHECKED__ */
 {
   void **p = (void **)ptr;
@@ -261 +261 @@
 void mutt_str_replace (char **p, const char *s)
 {
-  FREE (p);
+  FREE (p);             /* __FREE_CHECKED__ */
   *p = safe_strdup (s);
 }

menu.c

@@ -704 +704 @@
   }
 
-  FREE (p);
+  FREE (p);             /* __FREE_CHECKED__ */
 }
 

mh.c

@@ -223 +223 @@
   if ((*fp = fdopen (fd, "w")) == NULL)
   {
-    FREE (tgt);
+    FREE (tgt);         /* __FREE_CHECKED__ */
     close (fd);
     unlink (path);
@@ -489 +489 @@
     mutt_free_header (&(*md)->h);
 
-  FREE (md);
+  FREE (md);            /* __FREE_CHECKED__ */
 }
 

mutt_idna.c

@@ -90 +90 @@
   
  notrans:
-  FREE (out);
+  FREE (out);           /* __FREE_CHECKED__ */
   *out = safe_strdup (in);
   return 1;
@@ -115 +115 @@
   if (rv < 0)
   {
-    FREE (out);
+    FREE (out);         /* __FREE_CHECKED__ */
     *out = safe_strdup (in);
   }

mutt_ssl_gnutls.c

@@ -289 +289 @@
     gnutls_certificate_free_credentials (data->xcred);
     gnutls_deinit (data->state);
-    safe_free ((void **) &conn->sockdata);
+    FREE (&conn->sockdata);
   }
 

muttlib.c

@@ -292 +292 @@
   FREE (&(*h)->data);
 #endif
-  FREE (h);
+  FREE (h);             /* __FREE_CHECKED__ */
 }
 
@@ -680 +680 @@
   mutt_free_list (&(*p)->in_reply_to);
   mutt_free_list (&(*p)->userhdrs);
-  FREE (p);
+  FREE (p);             /* __FREE_CHECKED__ */
 }
 
@@ -914 +914 @@
           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;
       }
@@ -1394 +1394 @@
   else
   {
-    safe_free(&b->data);
+    FREE(&b->data);
   }
   memset(b, 0, sizeof(BUFFER));
@@ -1474 +1474 @@
    FREE(&(*p)->data);
    /* dptr is just an offset to data and shouldn't be freed */
-   FREE(p);
+   FREE(p);             /* __FREE_CHECKED__ */
 }
 
@@ -1546 +1546 @@
   regfree ((*pp)->rx);
   FREE (&(*pp)->rx);
-  FREE (pp);
+  FREE (pp);            /* __FREE_CHECKED__ */
 }
 
@@ -1573 +1573 @@
     *list = (*list)->next;
     mutt_free_regexp (&p->rx);
-    safe_free(&p->template);
+    FREE (&p->template);
     FREE (&p);
   }

mx.c

@@ -1527 +1527 @@
   }
 
-  FREE (msg);
+  FREE (msg);           /* __FREE_CHECKED__ */
   return (r);
 }

pager.c

@@ -390 +390 @@
     if ((*QuoteList)->prefix)
       FREE (&(*QuoteList)->prefix);
-    FREE (QuoteList);
+    FREE (QuoteList);           /* __FREE_CHECKED__ */
     *QuoteList = ptr;
   }

pgplib.c

@@ -184 +184 @@
   pgp_free_uid (&kp->address);
   FREE (&kp->keyid);
-  FREE (kpp);
+  /* mutt_crypt.h: 'typedef struct pgp_keyinfo *pgp_key_t;' */
+  FREE (kpp);           /* __FREE_CHECKED__ */
 }
 

remailer.c

@@ -127 +127 @@
   FREE (&(*r)->ver);
   
-  FREE (r);
+  FREE (r);             /* __FREE_CHECKED__ */
 }
 
@@ -217 +217 @@
     mix_free_remailer (&type2_list[i]);
   
-  FREE (type2_list);
+  FREE (type2_list);            /* __FREE_CHECKED__ */
 }
 

rfc1524.c

@@ -369 +369 @@
   FREE (&p->printcommand);
   FREE (&p->nametemplate);
-  FREE (entry);
+  FREE (entry);         /* __FREE_CHECKED__ */
 }
 

rfc2047.c

@@ -565 +565 @@
                   encode_specials ? RFC822Specials : NULL);
 
-  FREE (pd);
+  FREE (pd);            /* __FREE_CHECKED__ */
   *pd = e;
 }
@@ -762 +762 @@
   *d = 0;
 
-  FREE (pd);
+  FREE (pd);            /* __FREE_CHECKED__ */
   *pd = d0;
   mutt_str_adjust (pd);

rfc2231.c

@@ -184 +184 @@
     FREE (&(*p)->attribute);
     FREE (&(*p)->value);
-    FREE (p);
+    FREE (p);           /* __FREE_CHECKED__ */
   }
 }
@@ -365 +365 @@
     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;
   }

rfc822.c

@@ -793 +793 @@
 
 #ifdef TESTING
-int safe_free (void **p)
+int safe_free (void **p)        /* __SAFE_FREE_CHECKED__ */
 {
   free(*p);             /* __MEM_CHECKED__ */

sendlib.c

@@ -829 +829 @@
     FREE (&tcode[i]);
 
-  FREE (tcode);
+  FREE (tcode);         /* __FREE_CHECKED__ */
   
   return ret;
@@ -1820 +1820 @@
     {
       unlink (msg);
-      FREE (tempfile);
+      FREE (tempfile);          /* __FREE_CHECKED__ */
       _exit (S_ERR);
     }
@@ -1851 +1851 @@
       {
         unlink (*tempfile); /* no longer needed */
-        FREE (tempfile);
+        FREE (tempfile);                /* __FREE_CHECKED__ */
       }
     }
@@ -1861 +1861 @@
       {
         unlink (*tempfile);
-        FREE (tempfile);
+        FREE (tempfile);                /* __FREE_CHECKED__ */
       }
     }
@@ -1873 +1873 @@
       /* the parent is already dead */
       unlink (*tempfile);
-      FREE (tempfile);
+      FREE (tempfile);          /* __FREE_CHECKED__ */
     }