Wireshark-dev: Re: [Wireshark-dev] Malformed packets in CORBA protocol plugin
Date: Tue, 12 Dec 2006 11:13:30 +0000
> Andy.Ling@xxxxxxxxxxx wrote:
> 
> > Fine, but then why should it cope with an offset of 0. You shouldn't 
be
> > trying to add a protocol tree item when the offset is 0, there's
> > no data there either.
> 
> Eh?  An offset of 0 just means "beginning of the packet".
> 

Sorry. What I meant was offset & buffer length are equal. i.e we've
reached the end of the buffer and there is no more data.

> If you mean "length of 0", then a length of 0 is used in two places:
> 
>    1) generated items, where neither the offset nor the length are 
meaningful;
> 
>    2) the top-level item for a protocol if the amount of data available 
> for the protocol is 0 - that allows the top-level item to be added, with 

> the exception thrown when you create the sub-item.
> 

So there are other good reasons for accepting 0 length that I didn't know
about. That's why I need the help.

> > We could discuss the semantics of this all day, but it's not fixing
> > the problem. I have perfectly good CORBA packets that are showing
> > up as errors. I have spent a fair bit of time finding out why and
> > now I'd like some help to find the best way to fix it.
> > 
> > I agree with you that proto_tree_add_item should not be called if
> > there is nothing to add and I said as much in one of my earlier
> > posts, but the current code is based on the principle that this
> > can be done.
> 
> It's not as if somebody necessarily explicitly said "I'll write this on 
> the assumption that there are always parameters to a call" - it might 
> have been written without bearing in mind that there aren't (i.e., it's 
> not as if it's a Fundamental Design Decision).
> 

I'm not suggesting that was the design decision. It might be just luck 
that
it has worked so far. The point is, the only reason CORBA 1.0 requests
with no parameters don't generate errors it because the 0 length case
is not considered an error. Presumably if they had produced errors it
would have been fixed before now.

> I'd suggest moving the start_dissecting() calls into the routines that 
> process individual procedure requests and replies, and avoid generating 
> those calls if there are no items expected.
> 
> See, for example, genOperationRequest() in wireshark_gen.py; it could, 
> for example, start out by setting a flag to false and, before the 
> self.getCDR3() call, if the flag is false, put out the 
> start_dissecting() call and set the flag to true, so that the 
> start_dissecting() call is put out before the first parameter.

OK, that looks promising. I will investigate

Thanks

Andy Ling