On Sat, Jul 26, 2003 at 01:34:32AM -0700, Guy Harris wrote:
> I don't have time right now to rewrite it (and don't know when I will,
> if ever),
I did, however, find time to put in some crude checks to keep it from
crashing on *some* MEGACO packets, including the one you sent; I've
checked in a fix, and attached a patch.
The parser probably should be significantly changed, however, to
properly lexically analyze packets.
Index: plugins/megaco/packet-megaco.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/plugins/megaco/packet-megaco.c,v
retrieving revision 1.8
diff -c -r1.8 plugins/megaco/packet-megaco.c
*** plugins/megaco/packet-megaco.c 26 Jul 2003 04:51:08 -0000 1.8
--- plugins/megaco/packet-megaco.c 30 Jul 2003 06:28:31 -0000
***************
*** 282,287 ****
--- 282,298 ----
if (tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E') != -1 && tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E') < tvb_current_offset)
tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E');
+ if (tvb_current_offset == -1) {
+ ti = proto_tree_add_item(tree,proto_megaco,tvb, 0, -1, FALSE);
+ megaco_tree = proto_item_add_subtree(ti, ett_megaco);
+ proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ "Sorry, no \"=\" in this packet, I can't parse it");
+ return;
+ }
+ /*
+ * "tvb_previous_offset" will only be set if the corresponding
+ * "tvb_find_guint8()" didn't return -1, so it's not -1.
+ */
len = tvb_current_offset - tvb_previous_offset;
tvb_get_nstringz0(tvb,tvb_previous_offset,len+1,transaction);
***************
*** 290,295 ****
--- 301,313 ----
tvb_current_offset = tvb_find_guint8(tvb, tvb_offset,
tvb_len, '{');
+ if (tvb_current_offset == -1) {
+ ti = proto_tree_add_item(tree,proto_megaco,tvb, 0, -1, FALSE);
+ megaco_tree = proto_item_add_subtree(ti, ett_megaco);
+ proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ "Sorry, no \"{\" in this packet, I can't parse it");
+ return;
+ }
len = tvb_current_offset - tvb_offset;
***************
*** 333,341 ****
--- 351,369 ----
/* Find version */
tvb_previous_offset = tvb_find_guint8(tvb, 0,
tvb_len, '/') + 1;
+ if (tvb_previous_offset == -1) {
+ proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ "Sorry, no \"/\" in the MEGACO header, I can't parse this packet");
+ return;
+ }
tvb_current_offset = tvb_find_guint8(tvb, tvb_previous_offset,
tvb_len, ' ');
+ if (tvb_previous_offset == -1) {
+ proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ "Sorry, no \" \" after the \"/\" in the MEGACO header, I can't parse this packet");
+ return;
+ }
tokenlen = tvb_current_offset - tvb_previous_offset;
***************
*** 348,362 ****
/* Find transaction */
tvb_offset = tvb_find_guint8(tvb, 0,
tvb_len, ':');
- tvb_current_offset = tvb_find_guint8(tvb, 0,
- tvb_len, '=');
/* Transaction / TransactionResponseAck */
tvb_current_offset = tvb_find_guint8(tvb, 0,
tvb_len, '=');
tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, transaction[0]);
tokenlen = tvb_current_offset - tvb_previous_offset;
--- 376,395 ----
/* Find transaction */
tvb_offset = tvb_find_guint8(tvb, 0,
tvb_len, ':');
/* Transaction / TransactionResponseAck */
+ /* We did this earlier, so we know it doesn't fail */
tvb_current_offset = tvb_find_guint8(tvb, 0,
tvb_len, '=');
tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, transaction[0]);
+ if (tvb_previous_offset == -1) {
+ proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ "Sorry, no \"%c\" past the \":\" in this packet, I can't parse it",
+ transaction[0]);
+ return;
+ }
tokenlen = tvb_current_offset - tvb_previous_offset;
