Re: [PATCH net] netfilter: nf_conntrack_h323: fix double cursor advance in decode_int()
Pablo Neira Ayuso <[email protected]>
| Newsgroups | gmane.linux.network,gmane.comp.security.firewalls.netfilter.devel,gmane.linux.kernel |
|---|---|
| Message-ID | <aogvvRZuH8j650Oj@chamomile> |
On Fri, Aug 21, 2026 at 02:52:29PM +0800, luoqing wrote: > From: Qing Luo <[email protected]> > > 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: [email protected] > Assisted-by: LLM > Signed-off-by: Qing Luo <[email protected]> > --- > 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 >