[Already sent on April fool's day, but it seems to have been lost]
Guillaume Chazarain wrote:
Good point, because of the overflow, a valid packet (unlikely with
such a high length)
would have been rejected. Other than that, I don't see anything bad
happening.
So I changed the guint16 to gint.
And here is the latest patch with some other fixes: correct
add_message_data() prototype, and check
for negative data_len.
Thanks.
--
Guillaume
Index: epan/dissectors/packet-armagetronad.c
===================================================================
--- epan/dissectors/packet-armagetronad.c (révision 17780)
+++ epan/dissectors/packet-armagetronad.c (copie de travail)
@@ -55,7 +55,7 @@
#define ACK 1
/*
- * CVS as of 05/05/2005
+ * armagetronad-0.2.8.1
* The list in src/network/Makefile.in does not seem up to date,
* so the numbers and names were retrieved at runtime using the
* 'nDescriptor* descriptors[MAXDESCRIPTORS]' array
@@ -87,14 +87,19 @@
{51, "big_server"},
{52, "small_request"},
{53, "big_request"},
+ {54, "big_server_master"},
+ {55, "big_request_master"},
{60, "transfer config"},
{200, "Chat"},
{201, "ePlayerNetID"},
{202, "player_removed_from_game"},
+ {203, "Chat Client"},
{210, "eTimer"},
{220, "eTeam"},
{230, "vote cast"},
- {231, "Chat"},
+ {231, "Kick vote"},
+ {232, "Server controlled vote"},
+ {233, "Server controlled vote expired"},
{300, "gNetPlayerWall"},
{310, "game"},
{311, "client_gamestate"},
@@ -102,7 +107,7 @@
{321, "destinaton"},
{330, "gAIPlayer"},
{331, "gAITeam"},
- {340, "winzone"},
+ {340, "zone"},
{0, NULL}
};
@@ -112,7 +117,9 @@
/* For each message in the frame */
while (tvb_length_remaining(tvb, offset) > 2) {
- guint16 data_len = tvb_get_ntohs(tvb, offset + 4) * 2;
+ gint data_len = tvb_get_ntohs(tvb, offset + 4) * 2;
+ if (data_len < 0)
+ return FALSE;
#if 0
/*
@@ -138,18 +145,19 @@
}
static void
-add_message_data(tvbuff_t * tvb, gint offset, guint16 data_len,
+add_message_data(tvbuff_t * tvb, gint offset, gint data_len,
proto_tree * tree)
{
- guint16 *data = NULL;
+ guchar *data = NULL;
if (tree) {
- data = (guint16*)tvb_memcpy(tvb, ep_alloc(data_len), offset, data_len);
+ data = tvb_memcpy(tvb, ep_alloc(data_len + 1), offset, data_len);
+ data[data_len] = '\0';
}
if (data) {
- guint16 *ptr, *end = &data[data_len / 2];
- for (ptr = data; ptr != end; ptr++) {
+ guint16 *ptr, *end = (guint16*) &data[data_len];
+ for (ptr = (guint16*) data; ptr != end; ptr++) {
/*
* There must be a better way to tell
* Ethereal not to stop on null bytes
@@ -168,8 +176,6 @@
proto_tree_add_string(tree, hf_armagetronad_data, tvb, offset,
data_len, (gchar *) data);
-
- data = NULL;
} else
proto_tree_add_item(tree, hf_armagetronad_data, tvb, offset,
data_len, FALSE);
@@ -178,7 +184,7 @@
static gint add_message(tvbuff_t * tvb, gint offset, proto_tree * tree,
GString * info)
{
- guint16 descriptor_id, message_id, data_len;
+ gint descriptor_id, message_id, data_len;
proto_item *msg;
proto_tree *msg_tree;
const gchar *descriptor;
