Wireshark-bugs: [Wireshark-bugs] [Bug 7624] epan/dissectors/packet-per.c dissect_per_constrained

Date: Tue, 14 Aug 2012 12:23:59 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7624

--- Comment #3 from Jan de Jongh <jfcmdejongh@xxxxxxxxx> 2012-08-14 12:23:58 PDT ---
I'm afraid this bug cannot be easily reproduced through capture files alone
:-(. so I could use some advice on how to proceed...

I've created a test ASN.1-based protocol named testasn64 (UPER encoding);
listing of asn1/testasn64/testasn64.asn:

-- START EXAMPLE asn1/testasn64/testasn64.asn.
TestAsn1SixtyFour DEFINITIONS AUTOMATIC TAGS ::=
BEGIN
Integer8Unsigned ::= INTEGER(0..255)
Integer8Signed ::= INTEGER(-128..127)
Integer16Unsigned ::= INTEGER(0..65535)
Integer34Asymmetric ::= INTEGER(-69183..17179800000)
Integer64Unsigned ::= INTEGER(0..18446744073709551615)
TestString ::= IA5String(SIZE(0..15))
TestAsn64 ::= SEQUENCE
{
    i8u         Integer8Unsigned,
    i34a        Integer34Asymmetric,
    i16u        Integer16Unsigned,
    string1     TestString,
    i64u        Integer64Unsigned,
    string2     TestString
}
END
-- END EXAMPLE asn1/testasn64/testasn64.asn.

[To my knowledge a valid ASN.1 file...]

and glued it into Wireshark 1.8.1 (edited configure.in; some Makefile.am's;
Makefile.common's); subsequently, the build

./autogen.sh
./configure
cd asn1/testasn64
make
cd ../..
make
./wireshark

goes well, and indeed TESTASN64 shows up in the protocol list.

Without special measures, i.e., with the following testasn64.cnf file:

# START EXAMPLE
# testasn64.cnf
# testasn64 conformation file for Wireshark

#.MODULE_IMPORT

#.EXPORTS

#.PDU
TestAsn64

#END EXAMPLE

the asn2wrs script generates calls (partially in mistake!) for the INTEGER
types like this:

(example for Integer34Asymmetric taken from generated
epan/dissectors/packet-testasn64.c)

/* START EXAMPLE epan/dissectors/packet-testasn64.c. */
static int
dissect_testasn64_Integer34Asymmetric
  (tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_,
   proto_tree *tree _U_, int hf_index _U_) {

  offset = dissect_per_constrained_integer(tvb, offset, actx, tree, hf_index,
             -69183,
             G_GINT64_CONSTANT(17179800000U), NULL, FALSE);

  return offset;

}
/* END EXAMPLE epan/dissectors/packet-testasn64.c. */

Because the range of Integer34Asymmetric does no fit into 32 bits, this is an
asn2wrs bug in itself (I'll try to submit a separate bug report for this
later...). The dissect_per_constrained_integer (in packet-per.c) expects 32-bit
values for the min and max parameters:

guint32
dissect_per_constrained_integer(tvbuff_t *tvb, guint32 offset, asn1_ctx_t
*actx, proto_tree *tree, int hf_index, guint32 min, guint32 max, guint32
*value, gboolean has_extension)

so the asn2wrs-generated code cannot be right (again, this is a somewhat
unrelated bug in itself). However, this can be circumvented (at least until
asn2wrs has been fixed for this particular case) as follows in testasn64.cnf:

# START EXAMPLE testasn64.cnf
# testasn64.cnf
# testasn64 conformation file for Wireshark

#.MODULE_IMPORT

#.EXPORTS

#.PDU
TestAsn64

#.FN_BODY Integer34Asymmetric
        gint64 value;
        guint64 INTEGER34_MIN = G_GINT64_CONSTANT(-69183);
        guint64 INTEGER34_MAX = G_GINT64_CONSTANT(17179800000);
        header_field_info *hfi = proto_registrar_get_nth (hf_index);
        hfi->type = FT_INT64;
        offset = dissect_per_constrained_integer_64b
          (tvb, offset, actx, tree, hf_index, INTEGER34_MIN, INTEGER34_MAX,
(guint *)(&value), FALSE);
        return offset;

#.FN_BODY Integer64Unsigned
        guint64 value;
        guint64 INTEGER64_MIN = G_GINT64_CONSTANT(0x0000000000000000);
        guint64 INTEGER64_MAX = G_GINT64_CONSTANT(0xffffffffffffffff);
        header_field_info *hfi = proto_registrar_get_nth (hf_index);
        hfi->type = FT_UINT64;
        offset = dissect_per_constrained_integer_64b
          (tvb, offset, actx, tree, hf_index, INTEGER64_MIN, INTEGER64_MAX,
(guint *)(&value), FALSE);
        return offset;

#.END
# END EXAMPLE testasn64.cnf

replacing the calls to dissect_per_constrained_integer with calls to
(presumably, experimental) dissect_per_constrained_integer_64b when needed
(i.e., when the range exceeds the 32-bit range, i.c., for Integer34Asymmetric
and Integer64Unsigned).

=== THIS IS WHERE THE PROBLEMS START ===

In order to highlight the problem, let me focus at Integer34Asymmetric and
focus at what happens in this particular case in
dissect_per_constrained_integer_64b.

First, the range for Integer34Asymmetric = 0x0000 0002 0000 0000.

Note that all 32 least-significant bits are zero here...

Since there are no ASN.1 extensions, the relevant code revealing the problem
looks like this, with range (defined as 64-bits) = max-min+1 = 0x0000 0004 0000
0000 (in other words, the lower 32 bits all being zero):

/* BEGIN EXAMPLE epan/dissectors.packet-per.c. */
        } else if((range<=255)||(!actx->aligned)) {
                /* 10.5.7.1
                 * 10.5.6       In the case of the UNALIGNED variant the value
("n" - "lb") shall be encoded
                 * as a non-negative  binary integer in a bit field as
specified in 10.3 with the minimum
                 * number of bits necessary to represent the range.
                 */
                char *str;
                int i, bit, length;
                guint32 mask,mask2;
                /* We only handle 32 bit integers */
                mask  = 0x80000000;
                mask2 = 0x7fffffff;
                i = 32;
                while ((range & mask)== 0){
                        i = i - 1;
                        mask = mask>>1;
                        mask2 = mask2>>1;
                }
                if ((range & mask2) == 0)
                        i = i-1;

                num_bits = i;
                length=1;
                if(range<=2){
                        num_bits=1;
                }

and the while-loop will never end, since 'range' has all lowest 32 bits zero...
The loop will never end... For me, an immediate problem is the definition of
mask and mask2 as guint32. This is an error, since we are dealing with 64-bit
numbers here.

My suggested patch solves at least this problem. (Though there are more...)

Please advice how to proceed with this. All I can do is supply patches that
show the current implementation does not work (leads to infinite loop) and my
patched solution works (for some cases...);

BR

-- JdJ

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.