Wireshark · Ethereal-dev: [Ethereal-dev] dcerpc and samr patches

[Todd Sabin <tsabin@xxxxxxxxxxxxx>]

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

Index: packet-dcerpc-samr.c
===================================================================
RCS file: /cvsroot/ethereal/packet-dcerpc-samr.c,v
retrieving revision 1.9
diff -u -r1.9 packet-dcerpc-samr.c
--- packet-dcerpc-samr.c	2002/02/10 02:23:17	1.9
+++ packet-dcerpc-samr.c	2002/02/10 20:28:21
@@ -1898,6 +1898,18 @@
 
 
 static int
+samr_dissect_lookup_domain_rqst(tvbuff_t *tvb, int offset, 
+                             packet_info *pinfo, proto_tree *tree,
+                             char *drep)
+{
+    offset = dissect_ndr_ctx_hnd (tvb, offset, pinfo, tree, drep,
+                                  hf_samr_hnd, NULL);
+    offset = dissect_ndr_nt_UNICODE_STRING (tvb, offset, pinfo, tree, drep,
+                                            hf_samr_domain);
+	return offset;
+}
+
+static int
 samr_dissect_lookup_domain_reply(tvbuff_t *tvb, int offset, 
                              packet_info *pinfo, proto_tree *tree,
                              char *drep)
@@ -3306,7 +3318,7 @@
 		samr_dissect_context_handle,
 		samr_dissect_rc },
         { SAMR_LOOKUP_DOMAIN, "LOOKUP_DOMAIN",
-		samr_dissect_get_domain_password_information_rqst,
+		samr_dissect_lookup_domain_rqst,
 		samr_dissect_lookup_domain_reply },
         { SAMR_ENUM_DOMAINS, "ENUM_DOMAINS",
 		samr_dissect_enum_domains_rqst,
Index: packet-dcerpc.c
===================================================================
RCS file: /cvsroot/ethereal/packet-dcerpc.c,v
retrieving revision 1.30
diff -u -r1.30 packet-dcerpc.c
--- packet-dcerpc.c	2002/02/08 11:02:03	1.30
+++ packet-dcerpc.c	2002/02/10 20:28:21
@@ -1848,6 +1848,16 @@
 	}
 
 	value=g_hash_table_lookup(dcerpc_matched, (void *)pinfo->fd->num);
+        if (!value) {
+            v.uuid = hdr.if_id;
+            v.ver = hdr.if_ver;
+            v.opnum = hdr.opnum;
+            v.req_frame = pinfo->fd->num;
+            v.rep_frame = -1;
+            v.max_ptr = 0;
+            v.private_data=NULL;
+            value = &v;
+        }
 
 	di.conv = conv;
 	di.call_id = hdr.seqnum;
@@ -1876,23 +1886,24 @@
 	}
 
 	value=g_hash_table_lookup(dcerpc_matched, (void *)pinfo->fd->num);
+        if (!value) {
+            v.uuid = hdr.if_id;
+            v.ver = hdr.if_ver;
+            v.opnum = hdr.opnum;
+            v.req_frame=-1;
+            v.rep_frame=pinfo->fd->num;
+            v.private_data=NULL;
+            value = &v;
+        }
 
 	di.conv = conv;
 	di.call_id = 0; 
 	di.smb_fid = -1;
 	di.request = FALSE;
-	if(value) {
-		di.call_data = value;
-	} else {
-		v.uuid = hdr.if_id;
-		v.ver=hdr.if_ver;
-		v.req_frame=-1;
-		v.rep_frame=pinfo->fd->num;
-		v.private_data=NULL;
-		di.call_data = &v;
-	}
+        di.call_data = value;
+
 	dcerpc_try_handoff (pinfo, tree, dcerpc_tree, tvb, offset,
-			hdr.opnum, FALSE, hdr.drep, &di);
+                            value->opnum, FALSE, hdr.drep, &di);
         break;
     }