On Friday 10 December 2010, David Woodhouse wrote: > On Fri, 2010-12-10 at 09:26 +0100, Stefan Fritsch wrote: > > I have found something else that looks fishy, but I don't know if > > it can be triggered in practice: > > > > internal_lsearch_find() in src/lookups/lsearch.c sets store_pool > > to MAIN_POOL but fails to reset store_pool to old_pool when > > returning DEFER in line 199. Maybe this could lead to another > > function later wrongly resetting the MAIN_POOL instead of the > > SEARCH_POOL which could probbably result in memory corruption. > > I think that's fine. It's only ever called from > internal_search_find(), which is itself overriding store_pool to > point to POOL_SEARCH before calling it, then setting it back again > to whatever it was before.
True. So it's not an issue right now. > The only reason internal_lsearch_find() worries about old_pool is > for its own purposes, because it wants to use some temporary space > on POOL_MAIN and then switch back to SEARCH_POOL (or whatever pool > its called had selected for it). I was worried about constructs like this old_pool = store_pool; store_pool = SEARCH_POOL; reset_point = store_get(0); ... internal_search_find(...); ... store_reset(reset_point); store_pool = old_pool; In this case, the store_reset would reset MAIN_POOL, which is not what the caller of internal_search_find wanted to do. But after reading the store code some more, I think that store_reset() would cause exim to exit in this case, so it could only cause a DoS. But you may want to fix internal_lsearch_find() anyway, in order to prevent future bugs and to make it clearer in the code that there really is no problem. Cheers, Stefan -- ## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##
