[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

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

[Pablo Neira Ayuso]

On Wed, Jul 22, 2026 at 09:57:14PM +0900, Sangho Lee wrote:
> 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) {

This branch is now dead code after your update.

>  			v <<= bs->bit;
> -- 
> 2.43.0