Ethereal-dev: Re: [Ethereal-dev] dcerpc and samr patches

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: "Ronnie Sahlberg" <sahlberg@xxxxxxxxxxxxxxxx>
Date: Mon, 11 Feb 2002 15:09:10 +1100
Hi,

Attached patch fixes a few bugs.
LookupRids had a bug where I thought the IDL file specified an array of
pointers to longs where it should
have been a pointer to an array of longs.
I will go through the rest of the idl file later today and check if i did
the same mistake somewhere else as well.

There was another bug in dissect_ndr_uint64 in that the length specified to
proto_tree_add_item was 4 and not 8.
proto_tree_add_item did apparently not like that.

I also added three variables to the di structure so that the offset into the
tvb for the array headers coulb be remembered between
when they were dissected from the tvb until they were added to the display
tree.

Thanks a million for the captures, I will use them to look through all
packets in them to see what other bugs can be
found in samr.

Todd,
the ep* capture shows SAMR running atop UDP, but in the SAMR packets, there
are some 20 extra bytes remaining after the SAMR
PDU, are these ethernet trailers or is it used to authenticate the DCERPC
pdu?
They are not dissected anyway.



----- Original Message -----
From: "Todd Sabin"
To: "Guy Harris" , "Ronnie Sahlberg"
Sent: Monday, February 11, 2002 8:36 AM
Subject: [Ethereal-dev] dcerpc and samr patches


>
> Hi,
>
> The attached patch fixes a couple things in dcerpc and samr.
>
> o dissect dcerpc UDP replies correctly.  Currently, it uses the opnum
> from the reply packet.  This is frequently wrong in MS's
> implementation, so the patch fixes it to use the opnum from the
> request.  (I think it used to work this way before the dcerpc
> matching code went in, but I'm not sure.)
>
> o don't crash if the packet isn't found in the hash tables.  If a
> dcerpc packet is encapsulated in an ICMP redirect, e.g., the dcerpc
> layer won't be called on the first pass over the capture.  If the
> packet is then selected, there will be no entry in the hash tables for
> it, which can cause ethereal to coredump.  The patch dummies up an
> entry based on the packet.
>
> o dissect SamrLookupDomain requests properly.  The current dissector
> is incorrect.
>
>
> Also, the dissector for SamrLookupRids reply is slightly wrong.  I
> don't have a patch for that, but here's why it's getting it wrong:
> The first out parameter is [out] NAMES *pNames, where NAMES is
>
> typedef struct _NAMES {
>     ULONG Count;
>     [size_is (Count)] UNICODE_STRING *Names;
> } NAMES;
>
> So, the first thing is a top-level [ref] pointer, which is handed to
> dissect_ndr_pointer.  It knows there is no wire representation for top
> level [ref] pointers, so it tries to dissect the referent.  That
> _should_ dissect Count, and then dissect_ndr_pointer for Names.  But
> it first is checking for conformant data.  There isn't any at this
> level, but it thinks that there is, so everything ends up shifted by
> 4 bytes.  :(  Not sure exactly how to fix that right now...
>
> Guy and Ronnie, I'm sending captures which illustrate the above in a
> separate mail.  Feel free to share them with others, I just don't want
> to spam the list.  They are a small sniff with an ICMP redirected
> dcerpc call, and a largish sniff showing EPM and SAMR done over
> UDP...
>
>
> Todd
>
>

Attachment: dcerpc_lookuprids.diff
Description: Binary data