[PATCH net] netfilter: nf_conntrack_h323: fix double cursor advance in decode_int()

[PATCH net] netfilter: nf_conntrack_h323: fix double cursor advance in decode_int()

[luoqing]

From: Qing Luo <luoqing@kylinos.cn>

In the CONS case of decode_int(), when the field is actually decoded
(base is not NULL and the field carries the DECODE attribute),
get_uint() already advances the bitstream cursor by 'len' bytes.  The
subsequent unconditional "bs->cur += len" then advances the cursor a
second time over the same data, so the parser skips 'len' extra bytes.

All fields following such an integer are decoded from a wrong bit
position.  This affects the timeToLive field in RAS
RegistrationRequest/RegistrationConfirm messages: subsequent fields
(e.g. transport addresses) end up being parsed from arbitrary offsets
inside the message, so the helper may create expectations based on
bogus addresses/ports.

Only advance the cursor manually when get_uint() was not called, so
that the value bytes are consumed exactly once in either case.

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/netfilter/nf_conntrack_h323_asn1.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
index 6830c9da3507..a51275cc4d0f 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -336,8 +336,9 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
 			unsigned int v = get_uint(bs, len) + f->lb;
 			PRINT(" = %u", v);
 			*((unsigned int *)(base + f->offset)) = v;
+		} else {
+			bs->cur += len;
 		}
-		bs->cur += len;
 		break;
 	case UNCO:
 		BYTE_ALIGN(bs);
-- 
2.25.1

Re: [PATCH net] netfilter: nf_conntrack_h323: fix double cursor advance in decode_int()

[Pablo Neira Ayuso]

On Fri, Aug 21, 2026 at 02:52:29PM +0800, luoqing wrote:
> From: Qing Luo <luoqing@kylinos.cn>
> 
> In the CONS case of decode_int(), when the field is actually decoded
> (base is not NULL and the field carries the DECODE attribute),
> get_uint() already advances the bitstream cursor by 'len' bytes.  The
> subsequent unconditional "bs->cur += len" then advances the cursor a
> second time over the same data, so the parser skips 'len' extra bytes.
> 
> All fields following such an integer are decoded from a wrong bit
> position.  This affects the timeToLive field in RAS
> RegistrationRequest/RegistrationConfirm messages: subsequent fields
> (e.g. transport addresses) end up being parsed from arbitrary offsets
> inside the message, so the helper may create expectations based on
> bogus addresses/ports.
> 
> Only advance the cursor manually when get_uint() was not called, so
> that the value bytes are consumed exactly once in either case.

This patch should be targeted at nf-next.

Please, use [PATCH nf-next] given this does not result in a crash.

> Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
>  net/netfilter/nf_conntrack_h323_asn1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c b/net/netfilter/nf_conntrack_h323_asn1.c
> index 6830c9da3507..a51275cc4d0f 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -336,8 +336,9 @@ static int decode_int(struct bitstr *bs, const struct field_t *f,
>  			unsigned int v = get_uint(bs, len) + f->lb;
>  			PRINT(" = %u", v);
>  			*((unsigned int *)(base + f->offset)) = v;
> +		} else {
> +			bs->cur += len;
>  		}
> -		bs->cur += len;
>  		break;
>  	case UNCO:
>  		BYTE_ALIGN(bs);
> -- 
> 2.25.1
>