Re: [PATCH nft] netlink: fix stack buffer overrun when emitting ranged expressions

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Mar 18, 2025 at 02:24:26PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Hi Florian,
> > 
> > On Fri, Mar 14, 2025 at 01:41:49PM +0100, Florian Westphal wrote:
> > > Included bogon input generates following Sanitizer splat:
> > > 
> > > AddressSanitizer: dynamic-stack-buffer-overflow on address 0x7...
> > > WRITE of size 2 at 0x7fffffffcbe4 thread T0
> > >     #0 0x0000003a68b8 in __asan_memset (src/nft+0x3a68b8) (BuildId: 3678ff51a5405c77e3e0492b9a985910efee73b8)
> > >     #1 0x0000004eb603 in __mpz_export_data src/gmputil.c:108:2
> > >     #2 0x0000004eb603 in netlink_export_pad src/netlink.c:256:2
> > >     #3 0x0000004eb603 in netlink_gen_range src/netlink.c:471:2
> > >     #4 0x0000004ea250 in __netlink_gen_data src/netlink.c:523:10
> > >     #5 0x0000004e8ee3 in alloc_nftnl_setelem src/netlink.c:205:3
> > >     #6 0x0000004d4541 in mnl_nft_setelem_batch src/mnl.c:1816:11
> > > 
> > > Problem is that the range end is emitted to the buffer at the *padded*
> > > location (rounded up to next register size), but buffer sizing is
> > > based of the expression length, not the padded length.
> > > 
> > > Also extend the test script: Capture stderr and if we see
> > > AddressSanitizer warning, make it fail.
> > > 
> > > Same bug as the one fixed in 600b84631410 ("netlink: fix stack buffer overflow with sub-reg sized prefixes"),
> > > just in a different function.
> > > 
> > > Apply same fix: no dynamic array + add a length check.
> > 
> > While at it, extend it for similar code too until there is a way to
> > consolidate this? See attachment.
> 
> Sure, I can merge your snippet and push it out, is that okay?

Please, do so and push it out. Thanks.