Re: A review of the latest Polipo commits

On Wed, Dec 23, 2009 at 04:04:06PM +0200, Jacob Appelbaum wrote:
> Juliusz Chroboczek wrote:
> > Hi Chris, hi to all,
> > 
> > Thanks a lot for your hard work on Polipo.  Here's my review of the
> > latest commits, in no particular order.
> > 
> > 
> > 1. "Add an extra sanity check to avoid memmove segfault"
> > 
> > Could you please clarify?  I don't understand this fix.  Both reqlen and
> > reqbegin are signed, so it's not a question of sign contagion, right?
> > 
> 
> The issue was that the call to memmove() wasn't sanity checked at all.
> 
> It was possible to cause the call to have a negative number and thus it
> would result in an exploitable crash. In this change, I tried to ensure
> that the number would be above zero. Hopefully this is a useful defense
> in depth change but perhaps it's a noop...
> 
> Best,
> Jacob
> 

I think the first commit I made avoids the particular integer
overflow caused by the exploit script on the Tor mailing list,
but it's quite likely that there are others lurking, and Jake's
patch should help detect them.

-- 
Christopher Davis
Mangrin Remailer Admin
PGP: 0x0F8DA163

------------------------------------------------------------------------------
This SF.Net email is sponsored by the Verizon Developer Community
Take advantage of Verizon's best-in-class app development support
A streamlined, 14 day to market process makes app distribution fast and easy
Join now and get one step closer to millions of Verizon customers
http://p.sf.net/sfu/verizon-dev2dev