[PATCH net] netfilter: nf_conntrack_h323: fix get_bitmap() overread

[PATCH net] netfilter: nf_conntrack_h323: fix get_bitmap() overread

[Sangho Lee]

get_bitmap() first consumes the complete bytes covered by the current bit
offset and the requested bitmap length.  For totals below 32 bits, it then
unconditionally reads one more byte for the partial-byte remainder.

When the total is already byte aligned, there is no remainder.  The
caller's boundary check correctly permits only the complete bytes, so the
extra load reads one byte past the supplied PER buffer.  A crafted Q.931
User-User IE reaches this with a 17-bit sequence extension bitmap starting
at bit offset 7.  An AddressSanitizer build of the decoder reports:

  heap-buffer-overflow in get_bitmap
  decode_seq
  decode_seq
  DecodeH323_UserInformation
  DecodeQ931

The same condition also excludes a total of exactly 32 bits from the
alignment shift, producing an incorrect bitmap for unaligned inputs.

Read a trailing byte only when there is a partial-byte remainder.  Include
the 32-bit total in the alignment branch.  This makes all start offsets
and bitmap lengths from 1 through 32 agree with a bit-by-bit reference
decoder.

The same input reaches q931_help(), DecodeQ931(), and decode_seq() through
an nftables Q.931 conntrack helper on current nf.git.  The helper's static
scratch buffer has tailroom, so in-kernel KASAN does not report this logical
packet-boundary overread.  A diagnostic check immediately before the load
does observe bs->cur == bs->end.  No crash, disclosure, or corruption has
been demonstrated.

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Cc: stable@vger.kernel.org
Signed-off-by: Sangho Lee <kudo3228@gmail.com>
---
 net/netfilter/nf_conntrack_h323_asn1.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 6830c9da3..482860d2c 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -228,8 +228,9 @@ static unsigned int get_bitmap(struct bitstr *bs, unsigned int b)
 		     bytes--, shift -= 8)
 			v |= (unsigned int)(*bs->cur++) << shift;
 
-		if (l < 32) {
-			v |= (unsigned int)(*bs->cur) << shift;
+		if (l <= 32) {
+			if (l & 7)
+				v |= (unsigned int)(*bs->cur) << shift;
 			v <<= bs->bit;
 		} else if (l > 32) {
 			v <<= bs->bit;
-- 
2.43.0