Wireshark-bugs: [Wireshark-bugs] [Bug 5391] [Patch] MS TDS protocol update
Date: Tue, 23 Nov 2010 07:28:41 -0800 (PST)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5391

--- Comment #12 from Jeff Morriss <jeff.morriss.ws@xxxxxxxxx> 2010-11-23 07:28:37 PST ---
(In reply to comment #11)
> I have one objection to your changes. The DISSECTOR_ASSERT_NOT_REACHED macros
> were in places, that the dissector indeed should never reach regardless of
> packet's payload.
> In the first case the switch must handle all data types, for which the
> dissector itself might have set plp = TRUE in dissect_tds_type_info().
> In the second case the inner switch must handle all data types that lead here
> from the outer switch in the first place.
> So there's no input dependency here.

<sigh> I should have looked closer.  I put them back in rev 35014.

> BTW the return statement in place of the second DISSECTOR_ASSERT_NOT_REACHED is
> indented with tabs, I'm not sure whether that conforms to Wireshark's
> formatting rules, so I'm just letting you know ;)

My editor uses tabs.  Sometimes I catch on that a file doesn't use tabs and try
to avoid putting tabs in (and instead press-n-hold the space key), other times
I don't.  I took out the tabs in the above-mentioned commit.

> As to the ReportedBoundsError, I thought this is the correct way of aborting
> dissection in case further parsing makes no sense. When we encounter a type
> that is either unknown or unsupported yet, further dissection has literally no
> chance of yielding any reasonable results. The dissector will just try to parse
> another RPC parameter from the middle of currently incompletely parsed
> parameter. That can't do any good.
> This could be justified if it was possible to just skip to the next parameter,
> but in TDS that's impossible, because the size of an RPC param is not known in
> advance, so it needs to be parsed fully and correctly to get to the next param.
> In case of dissect_tds_all_headers it's questionable whether to continue
> parsing, because in case total headers length differs from sum of headers'
> lengths, then at what offset should further dissection continue? Either choice
> is totally arbitrary and each might succeed in one type of protocol error and
> fail in another.
> 
> Please tell me what you think.

ReportedBoundsError is SUPPOSED to mean "the dissector read past the (reported)
end of the TVB."  It is frequently (mis-?)used because it gives the nice
malformed-packet indication.  In this case it sounds like it probably does make
sense, so I put all but one back.

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