Wireshark-bugs: [Wireshark-bugs] [Bug 7210] Buildbot crash output: fuzz-2012-04-27-376.pcap
Date: Fri, 27 Apr 2012 20:29:46 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7210

--- Comment #6 from Guy Harris <guy@xxxxxxxxxxxx> 2012-04-27 20:29:46 PDT ---
(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > "Exception Occurred" means the dissector did crash
> > 
> > No, it means that the dissector called a tvbuff routine to fetch some data and
> > the data wasn't actually present in the tvbuff; this either means
> > 
> >     1) the dissector is attempting to fetch data that is legitimately not
> > there, which is a dissector bug;
> 
> Yes.
> 
> >     2) the dissector is attempting to fetch data that is supposed to be there,
> > but that isn't there in the packet, which is *not* a dissector bug, it's a
> > malformed packet;
> 
> Most of the time (in my experience) this is due to the dissector not validating
> a length field, in which case it is more correct for the dissector to validate
> the field against tvb_reported_length() or tvb_reported_length_remaining(), add
> an expert info to the actual malformed field, and continue on to dissect as
> much of the rest of the packet as it can.

It can also be, for example, due to a packet of the form (note: the below
example is *NOT* C):

    int32 type;
    case type of

    0:
        int32 code;

    1:
        int32 value_code;
        int32 value;
    }

having a type value of 1 but only having one 32-bit integer following it. 
There's no length field involved there.  Fuzzing could well produce a packet of
that sort.

In the case of, for example, a TLV where the length is invalid, one could
probably debate whether the right thing to do is to quit as soon as you get an
invalid length, and not bother trying to dissect the value, or to keep
dissecting the TLV and dissect as much of the value as you can.

> > In most cases, it's 2, 3, or 4, which are, as noted, not dissector bugs, so
> > there's nothing to fix.
> 
> Fair enough. Is there a way to differentiate the error messages for 2 and 3
> (the way that 4 already is) so that it's easy to tell if it's a genuine bug or
> not?

To some degree, packet-frame.c already attempts to do that - see the "if
(pinfo->fragmented) {" test in show_reported_bounds_error().  I'm not sure
pinfo->fragmented is set in all the places where it should be, and I'm not sure
that, even if it is, that would catch all the cases where the problem is that
the packet wasn't reassembled.

A general solution might require that a tvbuff have, in addition to a "captured
length" field and an "on the wire" length field, a "this is how big it would
have been if it were reassembled" field, although for IPv4 reassembly, for
example, you'd still need pinfo->fragmented as none of the fragments indicate
how big the reassembled packet will be.

(And, in any case, that wouldn't be sufficient to distinguish between a bug and
a non-bug, unless one deems all occurrences of case 2 as bugs, which I don't do
- I'm unconvinced that *any* of them are.)

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.