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

[Pablo Neira Ayuso <pablo@netfilter.org>]

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