checked in,
god the code in that dissector is ugly.
On Thu, 7 Apr 2005 23:03:16 +0100, Neil Kettle <njk4@xxxxxxxxxx> wrote:
> 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.
>
>
> _______________________________________________
> Ethereal-dev mailing list
> Ethereal-dev@xxxxxxxxxxxx
> http://www.ethereal.com/mailman/listinfo/ethereal-dev
>