Guy Harris <guy@xxxxxxxxxxxx> wrote on 11/12/2006 18:50:43:
> 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"?
>
The sequence length is 0.
After the call to decode_ServiceContextList the difference
between offset and tvb->(reported_)length is 0
After the call to set_new_alignment offset gets set beyond
the end of the buffer. In my test case, by 4 bytes.
> > 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.
>
Except in this case it is not an error. So it is not always "obvious"
> > 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.
>
True.
> 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.
Now it's getting beyond my abilities with python etc. I'm not sure
if omniidl and the python back-end can detect methods with & without
arguments to call start_dissecting correctly. Which is why I'm asking
for some help ;-)
Thanks for the input
Andy Ling
