Wireshark-dev: Re: [Wireshark-dev] Malformed packets in CORBA protocol plugin

From: Guy Harris <guy@xxxxxxxxxxxx>
Date: Mon, 11 Dec 2006 10:50:43 -0800
Andy.Ling@xxxxxxxxxxx wrote:

In dissect_giop_request_1_2 there is some code that looks like :-

  decode_ServiceContextList(tvb, request_tree, &offset,
                  stream_is_big_endian, GIOP_HEADER_SIZE);

  /*
   * GIOP 1.2 Request body must fall on an 8 octet alignment, taking into
   * account we are in a new tvbuff, GIOP_HEADER_SIZE octets from the
   * GIOP octet stream start.
   */

  set_new_alignment(&offset, GIOP_HEADER_SIZE, 8);

When there are no arguments the service context list is empty
so the call to set_new_alignment may and usually does, set
the offset beyond the length of the buffer.

Is that "empty" as in "the service context is absent" or "empty" as in "the sequence length in the service context is 0"?

proto_tree_add_item is designed to cope with offset equal to
the buffer length, but not beyond it. So it throws.

It *is* designed to cope with offset beyond the buffer length - an offset beyond the buffer length is an obvious error, so it throws an exception.

A fairly "safe" fix is to change start_dissecting in wireshark_gen.py
to look like :-

static proto_tree *start_dissecting(tvbuff_t *tvb, packet_info *pinfo, proto_tree *ptree, int *offset) {

    proto_item *ti = NULL;
    proto_tree *tree = NULL;            /* init later, inside if(tree) */

    if (check_col(pinfo->cinfo, COL_PROTOCOL))
        col_set_str(pinfo->cinfo, COL_PROTOCOL, "Q_QUENTINV3");

    /*
     * Do not clear COL_INFO, as nothing is being written there by
     * this dissector yet. So leave it as is from the GIOP dissector.
     * TODO: add something useful to COL_INFO
     *  if (check_col(pinfo->cinfo, COL_INFO))
     *     col_clear(pinfo->cinfo, COL_INFO);
     */

+    if (tvb_length_remaining(tvb, *offset) < 0)
+      return NULL ;

If the idea is that, if there are no arguments, the dissector should quit, the problem is that, in that case, if there are *supposed* to be arguments, no exception will be thrown if they're missing, so no indication that a request that should have arguments will show up.

I would, instead, have start_dissecting() take a gboolean argument indicating whether the are any arguments to be dissected and, if so, have it return after setting the protocol column.