Wireshark mailing list archives
Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c
From: Jeff Morriss <jeff.morriss.ws () gmail com>
Date: Fri, 31 May 2013 13:33:46 -0400
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 <eapache () gmail com> wrote:On Thu, May 30, 2013 at 9:46 PM, <morriss () wireshark org> 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 ModifiedThank 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... EvanP.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.) ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Evan Huus (May 30)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Evan Huus (May 30)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Jakub Zawadzki (May 31)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Evan Huus (May 31)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Jeff Morriss (May 31)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Jakub Zawadzki (May 31)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Jeff Morriss (May 30)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Evan Huus (May 31)
- Re: [Wireshark-commits] rev 49644: /trunk/epan/ /trunk/epan/: proto.c Evan Huus (May 30)