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

[PATCH net-next v2] 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.

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