Re: [exim-dev] Remote root vulnerability in Exim

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