Re: [patch] courier-imap-4.13 imapd patch replacing malloc, strcat and strcpy with asprintf

[Peter Malone]

My bad regarding the wrong mailing list. Sorry!

I agree, there are other daemons available. This daemon is brutal though 
which was why I raised it.

I'll just go ahead and fork it and clean it up. They don't appear to 
have many releases so it should be easy enough for me to stay on top of it.


On 04/20/14 06:12, Stuart Henderson wrote:
On 2014/04/19 20:38, Peter Malone wrote:
Hi,

I'm using OpenBSD 5.5. courier-imap-4.13 is in the ports tree and it's
quite a mess. I started looking at it today with the hope of just
replacing some of the malloc,strcat & strcpy calls with asprintf, but it
became clear before long that there's lots more issues with this code.

Regardless, here's a patch which fixes some of the malloc,strcat &
strcpy spaghetti in imapd.c

I figure you guys are fairly busy with OpenSSL right now so if it's OK
with you I'll get working on the rest of this code.

I spoke with the courier-imap team and their response was less than
satisfactory - claiming asprintf code won't compile on non-linux
platforms.
First off, wrong mailing list, tech@ is not for ports discussions so
please follow-up there if you need to.

This is not the sort of thing we can reasonably handle in patches in
ports.  Other mail daemons are available...


--- imapd_orig.c Sat Apr 19 19:38:18 2014
+++ imapd.c Sat Apr 19 20:09:48 2014
@@ -343,9 +343,8 @@

if (q)
{
- r=malloc(strlen(q)+sizeof("/."));
- if (!r) write_error_exit(0);
- strcat(strcpy(r, q), "/.");
+ if(asprintf(&r, "%s%s", q, "/.") == -1)
+ write_error_exit(0);
if (access(r, 0) == 0)
{
free(r);
@@ -1373,11 +1372,9 @@

static char *alloc_filename(const char *mbox, const char *name)
{
-char *p=malloc(strlen(mbox)+strlen(name)+sizeof("/cur/"));
-
- if (!p) write_error_exit(0);
-
- strcat(strcat(strcpy(p, mbox), "/cur/"), name);
+ char *p;
+ if(asprintf(&p, "%s%s%s", mbox, "/cur/", name) == -1)
+ write_error_exit(0);
return (p);
}

@@ -1812,14 +1809,12 @@
{
#if EXPLICITDIRSYNC

- char *p=malloc(strlen(folder)+sizeof("/new"));
+ char *p;
int fd;
-
- if (!p)
+
+ if(asprintf(&p, "%s%s", folder, "/new") == -1)
write_error_exit(0);

- p=strcat(strcpy(p, folder), "/new");
-
fd=open(p, O_RDONLY);

if (fd >= 0)
@@ -1828,7 +1823,8 @@
close(fd);
}

- p=strcat(strcpy(p, folder), "/cur");
+ if(asprintf(&p, "%s%s", folder, "/cur") == -1)
+                 write_error_exit(0);

fd=open(p, O_RDONLY);

@@ -2212,12 +2208,10 @@
{
struct imapscaninfo minfo;

- char *p=malloc(strlen(new_path)+sizeof("/" IMAPDB));
+ char *p;
+ if(asprintf(&p, "%s%s", new_path, "/" IMAPDB) == -1)
+                write_error_exit(0);

- if (!p)
- write_error_exit(0);
-
- strcat(strcpy(p, new_path), "/" IMAPDB);
unlink(p);
free(p);
imapscan_init(&minfo);
@@ -2448,14 +2442,12 @@
}
return 0;
}
- q=malloc(strlen(p)+sizeof("/new"));
- if (!q)
+ if(asprintf(&q, "%s%s", p, "/new") == -1)
{
free(p);
maildir_aclt_list_destroy(aclt_list);
return -1;
}
- strcat(strcpy(q, p), "/new");
free(p);

if (maildir_aclt_list_add(aclt_list,
@@ -4216,11 +4208,10 @@
if (p && (p=maildir_shareddir(".", p+1)) != NULL)
{
int rc;
- char *q=malloc(strlen(p)+sizeof("/shared"));
-
- if (!q) write_error_exit(0);
-
- strcat(strcpy(q, p), "/shared");
+ char *q;
+
+ if(asprintf(&q, "%s%s", p, "/shared") == -1)
+ write_error_exit(0);
free(p);
rc=append(tag, tok->tokenbuf, q);
free(q);
@@ -6041,10 +6032,9 @@
isshared=0;
if (is_sharedsubdir(cqinfo.destmailbox))
{
- char *p=malloc(strlen(cqinfo.destmailbox)+sizeof("/shared"));
-
- if (!p) write_error_exit(0);
- strcat(strcpy(p, cqinfo.destmailbox), "/shared");
+ char *p;
+ if(asprintf(&p, "%s%s", cqinfo.destmailbox, "/shared") == -1)
+ write_error_exit(0);

free(mailbox);
mailbox=cqinfo.destmailbox=p;
--- imapd_orig.c        Sat Apr 19 19:38:18 2014
+++ imapd.c     Sat Apr 19 20:09:48 2014
@@ -343,9 +343,8 @@
  
  		if (q)
                {
-                       r=malloc(strlen(q)+sizeof("/."));
-                       if (!r) write_error_exit(0);
-                       strcat(strcpy(r, q), "/.");
+                       if(asprintf(&r, "%s%s", q, "/.") == -1)
+                               write_error_exit(0);
                        if (access(r, 0) == 0)
                        {
                                free(r);
@@ -1373,11 +1372,9 @@
  
  static char *alloc_filename(const char *mbox, const char *name)
  {
-char   *p=malloc(strlen(mbox)+strlen(name)+sizeof("/cur/"));
-
-       if (!p) write_error_exit(0);
-
-       strcat(strcat(strcpy(p, mbox), "/cur/"), name);
+       char *p;
+       if(asprintf(&p, "%s%s%s", mbox, "/cur/", name) == -1)
+               write_error_exit(0);
        return (p);
  }
  
@@ -1812,14 +1809,12 @@
  {
  #if EXPLICITDIRSYNC
  
-	char *p=malloc(strlen(folder)+sizeof("/new"));
+       char *p;
        int fd;
-
-       if (!p)
+       
+       if(asprintf(&p, "%s%s", folder, "/new") == -1)
                write_error_exit(0);
  
-	p=strcat(strcpy(p, folder), "/new");
-
        fd=open(p, O_RDONLY);
  
  	if (fd >= 0)
@@ -1828,7 +1823,8 @@
                close(fd);
        }
  
-	p=strcat(strcpy(p, folder), "/cur");
+       if(asprintf(&p, "%s%s", folder, "/cur") == -1)
+                 write_error_exit(0);
  
  	fd=open(p, O_RDONLY);
  
@@ -2212,12 +2208,10 @@
  {
  struct imapscaninfo minfo;
  
-	char *p=malloc(strlen(new_path)+sizeof("/" IMAPDB));
+       char *p;
+       if(asprintf(&p, "%s%s", new_path, "/" IMAPDB) == -1)
+                write_error_exit(0);
  
-	if (!p)
-               write_error_exit(0);
-
-       strcat(strcpy(p, new_path), "/" IMAPDB);
        unlink(p);
        free(p);
        imapscan_init(&minfo);
@@ -2448,14 +2442,12 @@
                        }
                        return 0;
                }
-               q=malloc(strlen(p)+sizeof("/new"));
-               if (!q)
+               if(asprintf(&q, "%s%s", p, "/new") == -1)
                {
                        free(p);
                        maildir_aclt_list_destroy(aclt_list);
                        return -1;
                }
-               strcat(strcpy(q, p), "/new");
                free(p);
  
  		if (maildir_aclt_list_add(aclt_list,
@@ -4216,11 +4208,10 @@
                        if (p && (p=maildir_shareddir(".", p+1)) != NULL)
                        {
                                int rc;
-                               char    *q=malloc(strlen(p)+sizeof("/shared"));
-
-                               if (!q) write_error_exit(0);
-
-                               strcat(strcpy(q, p), "/shared");
+                               char *q;
+                               
+                               if(asprintf(&q, "%s%s", p, "/shared") == -1)
+                                       write_error_exit(0);
                                free(p);
                                rc=append(tag, tok->tokenbuf, q);
                                free(q);
@@ -6041,10 +6032,9 @@
                isshared=0;
                if (is_sharedsubdir(cqinfo.destmailbox))
                {
-               char    *p=malloc(strlen(cqinfo.destmailbox)+sizeof("/shared"));
-
-                       if (!p) write_error_exit(0);
-                       strcat(strcpy(p, cqinfo.destmailbox), "/shared");
+                       char *p;
+                       if(asprintf(&p, "%s%s", cqinfo.destmailbox, "/shared") 
== -1)
+                               write_error_exit(0);
  
  			free(mailbox);
                        mailbox=cqinfo.destmailbox=p;