Hi,
Find enclosed a patch for the fcels dissector. I noticed that some constructed strings may
grow beyond the bounds of several buffers stored on the stack. Consequently, it is
possible
to construct several packets that will cause a buffer overflow, however, it is very
unlikely
that they are exploitable since we cannot control the overflow string.
For instance:
dissect_fcels_prlilo_payload, type == FC_TYPE_SCSI where flag = 0x30F3
causes a buffer of 100 bytes to be filled with 119+1 bytes.
It appears from compilation tests that the code is effectively 'saved' by the compiler in
that
the stack is padded to the extent that these problems may never have been noticed nor
likely to be noticed. However, better safe than sorry.
______________________________________________________________________
Neil K
(njk4@xxxxxxxxxx)
(mu-b@xxxxxxxxxxxxxx)
--- packet-fcels.orig 2005-03-10 15:53:43.000000000 +0000
+++ packet-fcels.c 2005-04-07 22:42:58.883373000 +0100
@@ -476,6 +476,9 @@
}
}
+/* Maximum length of possible string from, construct_*_string
+ * 296 bytes, FIX possible buffer overflow */
+#define FCELS_LOGI_MAXSTRINGLEN 512
static void
dissect_fcels_logi (tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree,
@@ -487,7 +490,7 @@
class;
proto_tree *logi_tree, *cmnsvc_tree;
proto_item *subti;
- gchar flagstr[256];
+ gchar flagstr[FCELS_LOGI_MAXSTRINGLEN];
guint16 flag;
if (tree) {
@@ -1064,6 +1067,10 @@
}
}
+/* Maximum length of possible string from, dissect_fcels_prlilo_payload
+ * 119 bytes, FIX possible buffer overflow */
+#define FCELS_PRLILO_MAXSTRINGLEN 256
+
static void
dissect_fcels_prlilo_payload (tvbuff_t *tvb, packet_info *pinfo _U_,
guint8 isreq, proto_item *ti, guint8 opcode)
@@ -1074,7 +1081,7 @@
proto_tree *prli_tree, *svcpg_tree;
int num_svcpg, payload_len, i, flag;
proto_item *subti;
- gchar flagstr[100];
+ gchar flagstr[FCELS_PRLILO_MAXSTRINGLEN];
/* We're assuming that we're invoked only if tree is not NULL i.e.
* we don't do the usual "if (tree)" check here, the caller must.
