On Thu, May 30, 2013 at 10:40 PM, Jeff Morriss <[email protected]> wrote: > On 05/30/2013 10:15 PM, Evan Huus 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! > > > You're welcome. It was really just a question of: > > 0) Finally getting annoyed enough at the problem. > 1) Biting the bullet and sitting down and staring at it for a while. > 2) Finally realizing it doesn't have to all be fixed at once (greatly > reducing the time of #1). > 3) Remembering (again) something Anders pointed out to me years ago at > Sharkfest: incremental improvements are better than no movement (and if you > start the movement, others may help you with the mess you've created ;-)). > And we can always back it out if it blows up horribly. :-)
Wise words. I'm looking forward to meeting everybody at Sharkfest this year :) >> 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... > > > Well, found one already (r49648)... Fortunately a minor one (in the grand > scheme). And one more that Jakub caught on code review which I fixed in r49652... ___________________________________________________________________________ > 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 ___________________________________________________________________________ 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
