Wireshark-bugs: [Wireshark-bugs] [Bug 6407] SDP dissector failure caused by slightly malformed s
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=6407
--- Comment #4 from Colin Caughie <c.caughie@xxxxxxxxxxxxxxxx> 2011-09-29 09:06:16 PDT ---
(In reply to comment #3)
> (In reply to comment #0)
> > We have observed certain SDP files describing H.264 streams, in which the
> > sprops-parameter-sets parameter is slightly malformed; namely the SPS NALU has
> > an extra zero byte appended to the end.
> >
> > This does not cause any problems to any real RTSP client that we have seen, but
> > Wireshark reports a malformed packet
>
> As it should. It's in the business of reporting protocol errors. This is one.
> it really doesn't matter in 'any real RTSP client' has no problem with it.
Point taken, however this "glossing over" behavior was already present in that
function - i.e. if there are any bytes left over after reading one NALU it
tries to interpret them as another NALU, even though as far as I can tell there
should only ever be a single NALU in the buffer passed to this function.
All I did was prevent the fatal errors caused by not checking the buffer length
correctly before doing this. IMO this was a coding error, not a protocol error.
However I'd be happy to fix the behavior of this function further as long as it
doesn't break intended behavior in other scenarios - i.e. raise an error if the
buffer isn't entirely consumed by the call to
dissect_h264_seq_parameter_set_rbsp().
> > and does not display any lines of the SDP file after the offending a=fmtp line.
>
> It should try to continue. Although protocol errors can lead to further
> misinterpretations down the line.
In this case we're processing the contents of self-contained, Base64-encoded
string inside the SDP packet. As far as I can see an error in that can't
possibly invalidate further lines of the SDP.
> > It turns out this is caused by a simple buffer underrun in packet-h264.c; in
> > dissect_h264_nal_unit(), tvb_get_bits32 is called without first checking that
> > there are at least 4 bytes left in the buffer.
>
> Hence the use of a TVB; it's safe that way, causing the malformed exception.
> Don't make it go away. Make it so the the dissector reports a protocol error
> and can continue. All this patch does is cover up the problem. Not nice.
Sounds like a good idea - can you give me some pointers on how to do this? I.e.
show an error in the contents of the embedded buffer but allow it to continue
decoding the rest of the SDP packet?
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
