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.
--- 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;