On 5/31/13 10:33 AM, Jeff Morriss wrote: > On 05/31/13 12:59, Jakub Zawadzki 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? :) > > It appears to have a fairly negligible effect. Here's a comparison > using tools/test-captures.sh (on a pool of files): > > > before: > real 5m41.759s > user 5m21.718s > sys 0m13.510s > > after: > real 5m39.395s > user 5m23.165s > sys 0m13.639s > > (That's about a 0.46% increase.) > > And here's one focusing just on the "no tree" case: > > before: > real 1m19.675s > user 1m6.997s > sys 0m10.238s > > after: > real 1m20.773s > user 1m7.913s > sys 0m10.387s > > (That's about a 1.36% increase.)
It's been fuzzing all weekend without any failures, so at the risk of inviting disaster I added r49644 & r49652. I'm hoping to release 1.10.0 on Wednesday (June 5th) and 1.8.8 + 1.6.16 on Friday (June 7). The 1.6 branch reaches EOL on Friday as well. ___________________________________________________________________________ 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
