Message-ID: <Pine.LNX.4.21.0610080412480.22104-100000@linuxbox.org>
Date: Sun, 8 Oct 2006 04:18:41 -0500 (CDT)
From: Gadi Evron <ge@...uxbox.org>
To: Alexander Sotirov <asotirov@...ermina.com>
Cc: full-disclosure@...ts.grok.org.uk, funsec@...uxbox.org,
	vuln-dev@...urityfocus.com, bugtraq@...urityfocus.com
Subject: Re: [funsec] Technical Paper on the ZERT Patch and VML [was: Re:
 ZERT patch for setSlice()]

On Wed, 4 Oct 2006, Alexander Sotirov wrote:
> Rewriting the entire function in asm is a lot of unnecessary effort. Why didn't
> you add a simple length check and a 5-byte jump to it in the vulnerable function?
> 
> Patch right before the call to _IE5_SHADETYPE_TEXT::TOKENS::Ptok, check the
> length of the string, and you're done. Or you can patch the copy loop and count
> the characters there. It's easier and safer than rewriting the function.

Hi. Thank you very much for your valued input.

What you say makes sense (patching by staging the size check somewhere in
the binary and then just adding a 5-byte jump/call to it before or within
the Ptok() function). We believe the less we overwrite, the less chance
there is that something will go wrong.

However, we did experiment with this. It is the approach we at ZERT took
with an earlier patch version in C that Michael Hale Ligh wrote, before
Gil Dabah's final version, and found that while it worked nicely on one
version of vgx.dll, it was a big problem making it universal for all
versions. The reason was that the one Michael tested with had an align 20
(20 free bytes) where the size-check could be staged. None of the other
DLLs had this available. Nor did the others have any free space at the end
of a segment. The tool we conventionally use to add new sections to PE
files produced an error because there wasn't enough room in the PE header
for a new section.

We started discussing solutions to get around this at the time, but then
instead of breaking the speed of light which some claim is impossible 
( :) ) we did what hackers do and just went to warp speed.

So, even if all versions had that align 20 of free space they would no
doubt be in different offsets of the file, so we would need a specific
configuration for each version, and this would be much messier than the
way we actually implemented the patch.

A good engineer doesn't blame his tools though, which sounds like what we
just did.

Limited space in the binary was the biggest issue, and we are working on
that being solved for future incidents we are called to respond to by
developing better tools to better utilize these slicks/gaps(/whatever you
call it) in the PE. This would allow for any amount of patch code to be
staged.

For a higher success rate and the purpose of being as generic as possible,
patches are supposed to be as small as possible.

Code crunching, however, is something which should not be discounted. In
our first patch (and especially in our unsupported systems patch) this is
very apparent. The effort was not unnecessary, and not really an effort
for a better quality of our work. Putting that aside, the original code
(before further development and QA) was done in less than 5
minutes. ROI-wise, it was worth it. Thank you though.

As before, we explain more of the process in the paper we released to the
public:
http://isotf.org/zert/papers/vml-details-20061004.pdf

	Gadi.

Powered by blists - more mailing lists