GNOME Bugzilla – Bug 447425
Evolution should support POP accounts to have their own storage/folder-trees
Last modified: 2012-03-02 13:10:33 UTC
Transcript from IRC: Some invalid reads (byref passing of NULL) are also happening. Especially in the POP code and the reason is non-locking also those are exploitable. Although a lot more difficult (you need to get evolution to race). If a disconnect happens with the right timing, compared to the LIST command that camel sends pe->engine will become NULL, and that one is used somewhere (which causes a crash). Since it'll be written (valgrind reported this), I think writing to NULL is exploitable. Just very difficult There are a lot of those in the POP code A valgrind warning fix (unrelated) http://tinymail.org/trac/tinymail/changeset/2143 This one fixes the locking in the POP code: http://tinymail.org/trac/tinymail/changeset/2142 This is another member that needs locking in the POP code: http://tinymail.org/trac/tinymail/changeset/2137 ps. Using svn diff -r rev:rev-1 it should be possible to get these changes as nice patches that can be used to merge upstream camel with these fixes.
probably easier to just use a GStaticMutex lock instead of *lock, then you don't have to malloc one. also, no need to use g_new0() and then g_static_mutex_init(), the init() will set the values correctly. g_new0()'ing there doesn't save the code from anything over the use of g_new()
Philip, can you provide a reworked patch as fejj suggested?
Varadhan is already working on bringing the entire POP3 code of camel-lite into an experimental but comittable status into upstream camel. More information: http://mail.gnome.org/archives/evolution-patches/2007-June/msg00006.html
About Comment #1: A non allocated lock would be a global one. In Tinymail at least I've built-in the possibility to have many POP accounts. Their code doesn't need to lock the others (it's a local lock for that instance). Or also: locking the data ... not the code. So although the type's name is misleading here, I'm not so sure that I agree fully with the non-allocated version of GStaticMutex. On the gnew0 vs. gnew: Yes, true. I usually use allocators that instantly set everything to 0. Mostly to ease debugging. On top of it I also usually yet initialize everything, for code readability. With nowadays mobile CPUs (and Desktop CPUs too, of course), it doesn't make a lot sense anymore (at least to me) to micro-optimize this. I don't know what Varadhan is planning, but I'll wait for his work on the POP3 code before reworking patches, to avoid duplication of (trivial) work.
"A non allocated lock would be a global one" : I misunderstood you, sorry. You probably meant a non-pointer in the struct. That, of course, works indeed too.
yes, that is what I meant :)
Varadhan: Can you share you POP3 plans here?
Bumping version to a stable release.
So once again something rotting (in this case because Varadhan never answered)?
duplicate of bug 273944 ?
Yeah, it's a dupe. *** This bug has been marked as a duplicate of bug 273944 ***