On Fri, May 31, 2013 at 12:59 PM, Jakub Zawadzki <[email protected]> wrote: > Hi, > > On Thu, May 30, 2013 at 10:23:05PM -0400, Evan Huus wrote: >> On Thu, May 30, 2013 at 10:15 PM, Evan Huus <[email protected]> wrote: >> > On Thu, May 30, 2013 at 9:46 PM, <[email protected]> wrote: >> >> http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=49644 >> >> >> >> User: morriss >> >> Date: 2013/05/30 06:46 PM >> >> >> >> Log: >> >> (Finally!) check in part of Didier's patch to fix >> >> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3290 >> >> (TRY_TO_FAKE_THIS_ITEM disables bounds errors): >> >> >> >> Before calling TRY_TO_FAKE_THIS_ITEM() check if the length given (or, in >> >> the case of FT_UINT_{STRING,BYTES}, the length we retrieve from the TVB) >> >> exceeds what's left in the TVB. >> >> >> >> Do this only for proto_tree_add_item() for now (it's the most commonly >> >> used >> >> and thus the biggest trouble maker in this area). >> >> >> >> Similar changes for other APIs will come later (if nothing blows up). >> >> Despite >> >> the fuzz failures this bug has caused I'm not sure about back-porting >> >> it... >> >> >> >> Directory: /trunk/epan/ >> >> Changes Path Action >> >> +28 -3 proto.c Modified >> > >> > Thank you for this! >> > >> > If we get through a round or two of fuzz-testing without any failures >> > I would really like to see this backported to every stable branch >> > (even 1.6). It closes an entire class of security vulnerabilities, and >> > while it is a fairly non-trivial behavioural change in a hot code >> > path, it is relatively short and clearly not doing anything too odd. >> > >> > Fingers crossed for no unexpected side-effects... >> > Evan >> >> P.S. I don't know when we're expecting 1.10 final but I think delaying >> it (if necessary) in order to include this fix is fully justified. > > Has anyone performed some benchmarks before/after this patch? :)
I've been too afraid :P However, security > performance. Protocols that want performance can still manually put blocks of proto_tree_ calls under if (tree), they just have to be sure they're not breaking anything when they do so. Still worth backporting :) ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
