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;