diff mbox series

[net,v3] netfilter: Add protection for bmp length out of range

Message ID 571b3e3f7191b5f67792d1090fc537bf4045c522.camel@mediatek.com (mailing list archive)
State Awaiting Upstream
Delegated to: Netdev Maintainers
Headers show
Series [net,v3] netfilter: Add protection for bmp length out of range | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/apply fail Patch does not apply to net-0

Commit Message

Lena Wang (王娜) March 5, 2024, 11:38 a.m. UTC
From: Lena Wang <lena.wang@mediatek.com>

UBSAN load reports an exception of BRK#5515 SHIFT_ISSUE:Bitwise shifts
that are out of bounds for their data type.

vmlinux   get_bitmap(b=75) + 712
<net/netfilter/nf_conntrack_h323_asn1.c:0>
vmlinux   decode_seq(bs=0xFFFFFFD008037000, f=0xFFFFFFD008037018,
level=134443100) + 1956
<net/netfilter/nf_conntrack_h323_asn1.c:592>
vmlinux   decode_choice(base=0xFFFFFFD0080370F0, level=23843636) + 1216
<net/netfilter/nf_conntrack_h323_asn1.c:814>
vmlinux   decode_seq(f=0xFFFFFFD0080371A8, level=134443500) + 812
<net/netfilter/nf_conntrack_h323_asn1.c:576>
vmlinux   decode_choice(base=0xFFFFFFD008037280, level=0) + 1216
<net/netfilter/nf_conntrack_h323_asn1.c:814>
vmlinux   DecodeRasMessage() + 304
<net/netfilter/nf_conntrack_h323_asn1.c:833>
vmlinux   ras_help() + 684
<net/netfilter/nf_conntrack_h323_main.c:1728>
vmlinux   nf_confirm() + 188
<net/netfilter/nf_conntrack_proto.c:137>

Due to abnormal data in skb->data, the extension bitmap length
exceeds 32 when decoding ras message then uses the length to make
a shift operation. It will change into negative after several loop.
UBSAN load could detect a negative shift as an undefined behaviour
and reports exception.
So we add the protection to avoid the length exceeding 32. Or else
it will return out of range error and stop decoding.

Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
Signed-off-by: Lena Wang <lena.wang@mediatek.com>
---
v3:
  - add "Fixes:" tag.
v2:
  - add length protecton for another get_bitmap call.
  - update commit message to trim stacktrace.
---
---
 net/netfilter/nf_conntrack_h323_asn1.c | 4 ++++
 1 file changed, 4 insertions(+)

 		*(unsigned int *)base = bmp;
@@ -589,6 +591,8 @@ static int decode_seq(struct bitstr *bs, const
struct field_t *f,
 	bmp2_len = get_bits(bs, 7) + 1;
 	if (nf_h323_error_boundary(bs, 0, bmp2_len))
 		return H323_ERROR_BOUND;
+	if (bmp2_len > 32)
+		return H323_ERROR_RANGE;
 	bmp2 = get_bitmap(bs, bmp2_len);
 	bmp |= bmp2 >> f->sz;
 	if (base)

Comments

Lena Wang (王娜) March 8, 2024, 1:24 a.m. UTC | #1
On Tue, 2024-03-05 at 19:25 +0800, lena wang wrote:
> From: Lena Wang <lena.wang@mediatek.com>
> 
> UBSAN load reports an exception of BRK#5515 SHIFT_ISSUE:Bitwise
> shifts
> that are out of bounds for their data type.
> 
> vmlinux   get_bitmap(b=75) + 712
> <net/netfilter/nf_conntrack_h323_asn1.c:0>
> vmlinux   decode_seq(bs=0xFFFFFFD008037000, f=0xFFFFFFD008037018,
> level=134443100) + 1956
> <net/netfilter/nf_conntrack_h323_asn1.c:592>
> vmlinux   decode_choice(base=0xFFFFFFD0080370F0, level=23843636) +
> 1216
> <net/netfilter/nf_conntrack_h323_asn1.c:814>
> vmlinux   decode_seq(f=0xFFFFFFD0080371A8, level=134443500) + 812
> <net/netfilter/nf_conntrack_h323_asn1.c:576>
> vmlinux   decode_choice(base=0xFFFFFFD008037280, level=0) + 1216
> <net/netfilter/nf_conntrack_h323_asn1.c:814>
> vmlinux   DecodeRasMessage() + 304
> <net/netfilter/nf_conntrack_h323_asn1.c:833>
> vmlinux   ras_help() + 684
> <net/netfilter/nf_conntrack_h323_main.c:1728>
> vmlinux   nf_confirm() + 188
> <net/netfilter/nf_conntrack_proto.c:137>
> 
> Due to abnormal data in skb->data, the extension bitmap length
> exceeds 32 when decoding ras message then uses the length to make
> a shift operation. It will change into negative after several loop.
> UBSAN load could detect a negative shift as an undefined behaviour
> and reports exception.
> So we add the protection to avoid the length exceeding 32. Or else
> it will return out of range error and stop decoding.
> 
> Fixes: 5e35941d9901 ("[NETFILTER]: Add H.323 conntrack/NAT helper")
> Signed-off-by: Lena Wang <lena.wang@mediatek.com>
> ---
> v3:
>   - add "Fixes:" tag.
> v2:
>   - add length protecton for another get_bitmap call.
>   - update commit message to trim stacktrace.
> ---
> ---
>  net/netfilter/nf_conntrack_h323_asn1.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/netfilter/nf_conntrack_h323_asn1.c
> b/net/netfilter/nf_conntrack_h323_asn1.c
> index e697a824b001..540d97715bd2 100644
> --- a/net/netfilter/nf_conntrack_h323_asn1.c
> +++ b/net/netfilter/nf_conntrack_h323_asn1.c
> @@ -533,6 +533,8 @@ static int decode_seq(struct bitstr *bs, const
> struct field_t *f,
>  	/* Get fields bitmap */
>  	if (nf_h323_error_boundary(bs, 0, f->sz))
>  		return H323_ERROR_BOUND;
> +	if (f->sz > 32)
> +		return H323_ERROR_RANGE;
>  	bmp = get_bitmap(bs, f->sz);
>  	if (base)
>  		*(unsigned int *)base = bmp;
> @@ -589,6 +591,8 @@ static int decode_seq(struct bitstr *bs, const
> struct field_t *f,
>  	bmp2_len = get_bits(bs, 7) + 1;
>  	if (nf_h323_error_boundary(bs, 0, bmp2_len))
>  		return H323_ERROR_BOUND;
> +	if (bmp2_len > 32)
> +		return H323_ERROR_RANGE;
>  	bmp2 = get_bitmap(bs, bmp2_len);
>  	bmp |= bmp2 >> f->sz;
>  	if (base)

Dear all,
the patch is not applied and it may be due to format error with
evolution. I will try to resend v4 version with git send-mail.

Could anyone help to change the v3 state to Change Requested?

Thanks
Lena
Florian Westphal March 8, 2024, 3:55 p.m. UTC | #2
Lena Wang (王娜) <Lena.Wang@mediatek.com> wrote:
> the patch is not applied and it may be due to format error with
> evolution. I will try to resend v4 version with git send-mail.

Its applied:

767146637efc ("netfilter: nf_conntrack_h323: Add protection for
bmp length out of range")
diff mbox series

Patch

diff --git a/net/netfilter/nf_conntrack_h323_asn1.c
b/net/netfilter/nf_conntrack_h323_asn1.c
index e697a824b001..540d97715bd2 100644
--- a/net/netfilter/nf_conntrack_h323_asn1.c
+++ b/net/netfilter/nf_conntrack_h323_asn1.c
@@ -533,6 +533,8 @@  static int decode_seq(struct bitstr *bs, const
struct field_t *f,
 	/* Get fields bitmap */
 	if (nf_h323_error_boundary(bs, 0, f->sz))
 		return H323_ERROR_BOUND;
+	if (f->sz > 32)
+		return H323_ERROR_RANGE;
 	bmp = get_bitmap(bs, f->sz);
 	if (base)