Ill-formed endianness-swapping functions in BinPAC

Description

The functions named "pac_swap" in aux/binpac/lib/binpac.h.in do not swap correctly the endianness of signed integers of length 2 or 4 bytes.
Erroneous results will occur when the parsed number is negative (before being swapped). When swapped, the result will be 0xf...fXX where 0xXX is the first byte parsed.

This issue currently affects the SMB and DNP3 analyzers and will affect any new analyzer that parses signed integers.

The core of this issue is that right-shifts were assumed to be logical whereas most compilers use arithmetic shifts, which means that right-shifting will keep the sign of the value shifted. To fix this issue coming from the methods pac_swap for int16 and int32, 2 lines have to be modified :

  • line 68 : replace "(

    x

    >>

    8

    )" by "((

    x

    >>

    8

    ) &

    0xff

    )"

  • line 78 : replace "(

    x

    >>

    24

    )" by "((

    x

    >>

    24

    ) &

    0xff

    )"

Additionally, left-shifts are guarantied to shift in zeros, therefore the following lines can be modified to improve performance :

  • lines 68 and 73 : replace "((

    x

    &

    0xff

    ) <<

    8

    )" by "(

    x

    <<

    8

    )"

  • lines 81 and 89 : replace "((

    x

    &

    0xff

    ) <<

    24

    )" by "(

    x

    <<

    24

    )"

Environment

None

Assignee

Unassigned

Reporter

llh

Labels

External issue ID

None

Components

Fix versions

Priority

Normal
Configure