GNOME Bugzilla – Bug 447414
Security bug in Camel's IMAP provider
Last modified: 2013-09-14 16:49:49 UTC
The "SEQUENCE" value in the GData of the IMAP code (camel-imap-folder.c) is converted from a string using strtol. This allows for negative values. The imap_rescan uses this value as an int. It checks for !seq and seq>summary.length. It doesn't check for seq < 0. Although seq is used as the index of an array. This means that a negative index number can be fed to the array lookup by altering the output of an IMAP server. I'm marking this as a blocker (very very serious) security bug as this is remotely exploitable (I can put shell code in the UID field of the IMAP code, and make it execute on the victim's computer, as at the seq'd field of the index a g_strdup of the UID is written to memory. By carefully calculating the negative value and overwriting the instruction pointer near the array's start, I can let it point to that memory and get it to execute). I first informed the Camel authors about this bug, but they didn't respond quickly enough (it has been months now). I hereby stop caring about the secrecy of security bug reports and I do report it now. This bug affects nearly all versions of Evolutions. It can be fixed by either checking for seq < 0 or by using strtoul in stead of strtol.
Created attachment 89932 [details] [review] Patch that fixes this problem I'm doing both, changing strtol to strtoul and the extra check (although that one should not really be necessary anymore one using strtoul).
Created attachment 89933 [details] [review] Added a ChangeLog entry Please let me know if I can commit this
Phillip, patch looks good and please commit it to both STABLE and SVN trunk. Thanks.
If I can alter the output of IMAP server, by having a shell-script as UID (and executing remotely, VUl, etc.) I wonder why can I not do the same if the SEQUENCE is +ve ?
Sankar: You can, but it will just be a (long) UID. If the SEQUENCE is negative, then you will instruct your machine to also write that data at a random location (random depending on the negative value). It might then also be possible to let an instruction pointer point to that random location. If you then invoke that instruction pointer, on an architecture that does not protects the system from running code that isn't marked as executable ... it'll just run that (basics of a buffer overflow). Note, Veerapuram, that I'm not finding a STABLE branch: $ svn co svn+ssh://pvanhoof@svn.gnome.org/svn/evolution-data-server/STABLE evolution-data-server svn: URL 'svn+ssh://pvanhoof@svn.gnome.org/svn/evolution-data-server/STABLE' doesn't exist $ I already committed this to trunk, though.
does checking out from /svn/evolution-data-server/STABLE work? i'd rather use /svn/evolution-data-server/branches/gnome-2-18 for this...
is there a CVE for this issue?
not that I know, it's philip (the guy working on tinymail) that reported this bug while working on camel.
oops, wrong bugzilla, sorry for confusion
(In reply to comment #5) > Sankar: You can, but it will just be a (long) UID. If the SEQUENCE is negative, > then you will instruct your machine to also write that data at a random > location (random depending on the negative value). It might then also be > possible to let an instruction pointer point to that random location. If you > then invoke that instruction pointer, on an architecture that does not protects > the system from running code that isn't marked as executable ... it'll just run > that (basics of a buffer overflow). > > Note, Veerapuram, that I'm not finding a STABLE branch: > By STABLE, I meant the gnome-2-18 branch. Please commit it to gnome-2-18 branch as well. TIA.
(In reply to comment #0) > > I first informed the Camel authors about this bug, but they didn't respond > quickly enough (it has been months now). Though, I appreciate your efforts on solving this issue, but do not agree to your accusations. Camel authors are not working (fulltime) in Evolution and whatever they are doing currently is because of their interest and passion towards Evolution. AFAIK, you know who works on what component of evolution and IIRC, I never received any mail/IRC message about this - as I am the current maintainer of Camel. It would be better to think about more co-operative and constructive methods to work closely with the upstream than pointing fingers. As far as this bug is concerned, I would appreciate if you could search for more such buffer overflows and nail them. TIA. :-)
(In reply to comment #2) > Created an attachment (id=89933) [edit] > Added a ChangeLog entry > > Please let me know if I can commit this > BTW, though I accepted the patch - I am thinking of another approach, instead of checking for seq < 0, it would be better to change seq from int to unsigned int and use GPOINTER_TO_UINT in the first hunk and keep strtoul() and change seq from long to unsigned long. Phillip: Would you mind attaching a reworked patch?
(In reply to comment #12) > Phillip: Would you mind attaching a reworked patch? Varadhan, would you mind just doing it youself, if you prefer a slightly different approach? Since you are the current maintainer of Camel (see comment 11). Please, just fix this. Both in trunk and gnome-2-18 branch. Do attach the patches here, so packager can easily backport them. Also, I'd recommend posting a heads up to gnome.org distributor-list. This issue is publicly accessible [1] anyway. No secret. [1] I know of at least one distro who pushed a fix already, and announcing it on Bugtraq. There probably are more such distros and announcements already. Also, there are publicly accessible copies of this report in distribution specific bugracking systems.
according to the announcement of 2.11.4, this has been fixed in unstable and already committed, which is very good imo. http://svn.gnome.org/viewcvs/evolution-data-server/trunk/camel/providers/imap/camel-imap-folder.c?r1=7720&r2=7817 now can we please handle this like a security bug should be handled? thanks.
Andre, I have announced it to distributor-list@gnome.org already. Let me know, if anything else needs to be done.
(In reply to comment #14 and comment #15) Yes, fixing this in gnome-2-18 branch before 2.18.3 tarballs are due. Setting patch status 'committed', according to comment 5.
Will attach the fix in the morning. Its quite late here. ;-)
The patch in this bug report applies fine (only some fuzz) against 1.10.2, from the camel/ dir.
True, the patch in attachment 89933 [details] [review] applies just fine against the stable branch. I've added the fix to the GARNOME gnome-2-18 branch in SVN a few days ago.
I had to do some research to patch the oldstable Debian release (sarge), and it turns out this code is buggy since at least 2001 and quite probably since 2000! The patch applies against evolution 2.0.4 without problem.
Committed to stable.