Huge thanks to our Platinum Members Endace and LiveAction,
and our Silver Member Veeam, for supporting the Wireshark Foundation and project.

Wireshark-bugs: [Wireshark-bugs] [Bug 8919] New: patch: (packet-scsi-osd.c) Proper dissection of

Date: Wed, 10 Jul 2013 02:47:03 +0000
Bug ID 8919
Summary patch: (packet-scsi-osd.c) Proper dissection of responses with insufficient allocation length
Classification Unclassified
Product Wireshark
Version SVN
Hardware All
OS All
Status UNCONFIRMED
Severity Minor
Priority Low
Component Dissection engine (libwireshark)
Assignee [email protected]
Reporter [email protected]

Build Information:

--
The format of the Data In buffer is (see OSD2r4: section 4.15.3 OSD Data-In
Buffer format):
 * From offset 0 up to the allocated_length: parameter data
 * From retrieved attributes offset: get attributes segment

Currently the only service action that dissects the parameter data segment is
LIST. The implementation assumes that enough bytes have been allocated for the
parameter data segment, and dissects the parameter data by using the ADDITIONAL
LENGTH field in the response. However, the ADDITIONAL LENGTH field do not
reflect truncation of the parameter data when the allocation length is
insufficient (cf. section 5.2.2, 7.1.4.4, etc.), then we may inadvertently run
into the retrieved attributes segment.

Consider the case of a LIST command with an ALLOCATION LENGTH of 24, and a
RETRIEVED ATTRIBUTES OFFSET of 24 (i.e. there is no padding between parameter
data and retrieved attributes). The implementation will do:

    additional_length=tvb_get_ntoh64(tvb, offset); //offset==0

Since the parameter data will be truncated, the value of the additional length
field will be greater than 24-8. Then the implementation will read more than 16
"additional" bytes, hence running into the retrieved attributes segment.

Consider also the extreme case of a LIST command with an ALLOCATION LENGTH of
0. In that case there is no ADDITIONAL LENGTH field (because there is no
parameter data), and the "additional_length" will be read from the retrieved
attributes segment, giving an incorrect result.

Changes in dissect_osd_additional_length:
 * Change the function signature, add scsi_task_data_t* parameter.
 * Store the value of the ALLOCATION LENGTH field into cdata->itlq->alloc_len
 * If the allocation length overflows guint32, then itlq->alloc_len is assigned
with 0xFFFFFFFF (allocation length in OSD is an 8-byte value, but in that case
the PDU is likely malformed, or too large to be inspected manually).

Changes in dissect_osd_list:
 * Check the requested allocation_length *as well as* tvb_length_remaining
*and* additional_length before dissecting the parameter data.
 This verification is performed for both OSD-1 and OSD-2.

Sample captures (not the best examples, since *insufficient* allocation length
is not verified, but enough for demonstrating that the proposed patch doesn't
break the dissection):
 OSD-1: scsi-osd-example-001 in http://wiki.wireshark.org/SampleCaptures (there
are no packets with both additional length and retrieved attributes, but frames
69,70 show a request/response with sufficient allocation length)
 OSD-2: attachment 11155 [details]  (contains a request/response with sufficient
allocation length and retrieved attributes)


You are receiving this mail because:
  • You are watching all bug changes.