BER errors when parsing H248 (binary MEGACO)

Hello. 

I begin posts about possible bugs as you told me. 

The first problem I've encountered is the problem parsing binary MEGACO also
known as H248 protocol.



-----Original Message-----
From: Tarlovskij Eugene [mailto:[email protected]] 
Sent: Friday, May 19, 2006 8:11 PM
To: '[email protected]'
Subject: BER errors in H248 dissector (binary MEGACO)


I am developing a simple application which uses EPAN library and I have a
problem with H248 protocol (binary MEGACO). H248 dissector does not decode
packets.

BER decoder output looks like this (non-sense parts of message are skipped):

H.248 MEGACO
  mess
    version: 1
    messageBody: transactions (1)
        Item: transactionRequest (0)
            actions: 1 item
                contextId: Null Context(0)
                contextRequest
                BER Error: Wrong field in SEQUENCE  expected class:2
(CONTEXT) tag:3 but found class:2 tag:1
                BER Error: This field lies beyond the end of the known
sequence definition.

Captured packets are real data from working system so they are considered to
be valid. Short research of a packet and BER dissector revealed some
inconsistency in BER decoder.

Let's examine the dissection algorithm in details.

BER successfully parses contextId and tries to parse contextRequest (which
is optional field). 
First, it successfully finds contextRequest header and tries to construct
new TVB subset (see line 1138 of packet-ber.c). But the contextRequest field
is empty, so resulting TVB is zero bytes in length.
In line 1179 this TVB is passed to sub-parser which correctly parses 0 bytes
and returns 0 as a result, but zero return value from this function is
considered as error.
As a result, condition in line 1196 is true and BER decoder tries next
sequenced item. It leads to BER errors shown above.

So, here is the fix. 
I've just added (count!=length_remaining) to avoid treating zero-length
fields as parsing errors.

Please review the fix and tell we am I right. If you want me to send
captured packets simply ask me (I just do not want to litter this mailing
list with unnecessary binary data).

[ >>> begin DIFF <<< ] 

Index: packet-ber.c
===================================================================
--- packet-ber.c    (revision 18189)
+++ packet-ber.c    (working copy)
@@ -1193,7 +1193,7 @@
 #endif
        /* if it was optional and no bytes were eaten and it was */
        /* supposed to (len<>0), just try again. */
-       if((len!=0)&&(count==0)&&(seq->flags&BER_FLAGS_OPTIONAL)){
+
if((len!=0)&&(count==0)&&(count!=length_remaining)&&(seq->flags&BER_FLAGS_OP
TIONAL)){
            seq++;
            goto ber_sequence_try_again;
        /* move the offset to the beginning of the next sequenced item */

[ >>> end DIFF <<< ] 


Eugene Tarlovskij