Re: Two Possible Vulnerabilities in courier-imapd?

[Dan Anderson <dan-anderson () cox net>]

Hi,

I'll bite.

#1 is safe to free (I think that was what you were getting at) because it
was strdup'd
all_settings=strdup(all_settings);

#2 isn't a format string vulnerability because the tainted data from getenv
isn't used _as_ a format string (and we're lined up formats to args).
fprintf(stderr, "ERR: error storing a message, user=%s,
errno=%d\n",getenv("AUTHENTICATED"),
errno);
vs
fprintf(stderr, getenv("AUTHENTICATED"), errno);


Dan


On Sun, Apr 13, 2014 at 7:00 PM, Peter Malone <peter () petermalone org> wrote:

> Hi there,
>
> Lets take a look at two functions in courier-imap 4.15. The first one is
> emptytrash(), and the second one is store_mailbox().
>
>
> void emptytrash()
> {
>         char    *dir, *all_settings, *next_folder, *folder, *p;
>         unsigned l;
>
>         all_settings=getenv("IMAP_EMPTYTRASH");
>                return;
>
>         all_settings=strdup(all_settings);
>         if (!all_settings)
>                 return;
>
>         if (strchr(all_settings, ':') == 0 &&
>             strchr(all_settings, ',') == 0)
>         {
>                 l=atoi(all_settings);
>
>                 if (l <= 0)
>                         l=1;
>
>                 maildir_getnew(".", trash, NULL, NULL);
>                 if ((dir=maildir_folderdir(".", trash)))
>                 {
>                         maildir_purge(dir, l * 24 * 60 * 60);
>                         free(dir);
>                 }
>                 free(all_settings);
>                 return;
>         }
>
>         for (folder=all_settings; folder && *folder; )
>         {
>                 if (*folder == ',')
>                 {
>                         ++folder;
>                         continue;
>                 }
>                 next_folder=strchr(folder, ',');
>                 if (next_folder)
>                         *next_folder++=0;
>
>                 p=strchr(folder, ':');
>                 if (!p)
>                 {
>                         folder=next_folder;
>                         continue;
>                 }
>
>                 *p++=0;
>
>                 l=atoi(p);
>                 if (l <= 0)     l=1;
>
>                 maildir_getnew(".", folder, NULL, NULL);
>                 if ((dir=maildir_folderdir(".", folder)))
>                 {
>                         maildir_purge(dir, l * 24 * 60 * 60);
>                         free(dir);
>                 }
>                 folder=next_folder;
>         }
>         free(all_settings);
> }
>
> The interesting parts of this function are
> all_settings=getenv("IMAP_EMPTYTRASH");
> free(all_settings);
>
> Setting IMAP_EMPTYTRASH to something like "%s:%d%d%d%s%s%s" should cause
> the application to crash.
>
> Moving on to store_mailbox()... this function is too big to paste in
> this mail, but if we focus on lines 744 - 757:
>         if (fflush(fp) || ferror(fp))
>         {
>                 fprintf(stderr,
>                         "ERR: error storing a message, user=%s, errno=%d
> \n",
>                                 getenv("AUTHENTICATED"), errno);
>
>                 fclose(fp);
>                 unlink(tmpname);
>                 writes(tag);
>                 writes(nowrite);
>                 free(tmpname);
>                 free(newname);
>                 return (-1);
>         }
>
> I believe the above fprintf call is a format string vulnerability.
>
> I'm going to continue to look into this, however in the meantime I
> welcome your input regarding these two functions.
>
> Regards,
> Peter.
>
>
> _______________________________________________
> Sent through the Full Disclosure mailing list
> http://nmap.org/mailman/listinfo/fulldisclosure
> Web Archives & RSS: http://seclists.org/fulldisclosure/

_______________________________________________
Sent through the Full Disclosure mailing list
http://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: http://seclists.org/fulldisclosure/