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

Pablo Neira Ayuso <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.netdev,org.kernel.vger.netfilter-devel
Message-ID <aoOQHtBE1GNDaPou@chamomile>
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: [email protected]
> Signed-off-by: Sangho Lee <[email protected]>
> ---
>  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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.