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;

Reply via email to