Wireshark-bugs: [Wireshark-bugs] [Bug 8735] USB CCID dissector "runs off the rails" when trying
Tyson Key
changed
bug 8735
| What |
Removed |
Added |
| CC |
|
tyson.key@gmail.com
|
Comment # 3
on bug 8735
from Tyson Key
Weird, I'm sure that it worked, earlier. Since the USB CCID dissector doesn't
hook into the main USB dissector's descriptor-handling stuff (and I suspect
that dissector has its own bugs of this sort - I haven't looked at its code),
I'd expect the "GET DESCRIPTOR Response CONFIGURATION" packets to be marked as
malformed.
>From looking at that specific trace file (I tested with a different one that
only manifested the original issue, and forgot to upload it...), it looks like
I'd also need to fix the PC_to_RDR_XfrBlock handler; and maybe have a look at
some other portions of the code where I think that similar issues might occur.
Then again, the initial patch was designed only to fix the bug with
RDR_to_PC_DataBlock packets - but I'd be happy to roll another one.
The hf_ccid_dwLength thing seems weird - but I deliberately implemented it that
way, originally, since the spec
(http://www.usb.org/developers/devclass_docs/DWG_Smart-Card_CCID_Rev110.pdf)
says that the "dwLength" field is 4 bytes in size, and values are stored in
little-endian format.
I was going to combine the new length tests with the existing sub_selected
tests - but simply wrapping an (tvb_get_guint8(tvb, 1) != 0) around the code
that dealt with handovers seemed less redundant/more obvious, at the time. I'm
willing to try another approach, though
Thanks for the review.
(In reply to comment #2)
> Comment on attachment 10866 [details]
> A patch that checks to see if usbccid.dwLength == 0, before trying to
> dissect non-existent RDR_to_PC_DataBlock payloads
>
> Filtering on "malformed", I find 7 packets (72, 250, 512, 1878, 2864, 3196,
> and 3520). Applying the patch doesn't fix any of them (in fact it doesn't
> appear to even execute the code on those frames.
>
> Also, there seems to be some discrepancy with the size of hf_ccid_dwLength.
> It's a FT_UINT8, but all length parameters in its proto_tree_add_item are 4.
>
> I also think "next_tvb" could be checked in such a way to show there are no
> bytes in it (although the length check may be faster and need to be done
> before the next_tvb is setup). This should probably be done for all
> instances of where hf_ccid_dwLength is used to call a subdissector.
You are receiving this mail because:
- You are watching all bug changes.
