From: Willy Tarreau <w@1wt.eu>
To: David Laight <david.laight.linux@gmail.com>
Cc: Doruk Tan Ozturk <doruk@0sec.ai>,
oe-linux-nfc@lists.linux.dev, david+nfc@ixit.cz,
netdev@vger.kernel.org
Subject: Re: [PATCH] nfc: llcp: fix integer underflow and missing bounds checks in TLV parsing
Date: Wed, 3 Jun 2026 18:55:26 +0200 [thread overview]
Message-ID: <aiBcfsXn0BhBi5ae@1wt.eu> (raw)
In-Reply-To: <20260602135610.5f79ed10@pumpkin>
On Tue, Jun 02, 2026 at 01:58:55PM +0100, David Laight wrote:
> On Mon, 25 May 2026 22:24:27 +0200
> Doruk Tan Ozturk <doruk@0sec.ai> wrote:
>
> > Multiple out-of-bounds read vulnerabilities exist in the NFC LLCP TLV
> > parsers:
> >
> > 1. In nfc_llcp_recv_snl(), when an SDREQ TLV has length == 0,
> > service_name_len = length - 1 underflows to SIZE_MAX (size_t is
> > unsigned). The subsequent strncmp() and nfc_llcp_sock_from_sn()
> > calls then read unbounded kernel heap memory.
> >
> > 2. All LLCP TLV parsing loops (nfc_llcp_recv_snl, nfc_llcp_connect_sn,
> > nfc_llcp_parse_gb_tlv, nfc_llcp_parse_connection_tlv) read tlv[0]
> > and tlv[1] without first verifying that at least 2 bytes remain in
> > the buffer.
> >
> > A nearby malicious NFC device can trigger these without authentication --
> > LLCP link activation happens automatically after NFC-DEP.
> >
> > Fix by adding a minimum length check before the subtraction in the
> > SDREQ case, and adding bounds validation at the top of each TLV loop
> > iteration.
>
> Why not fix it properly so that it doesn't read beyond the end of the skb.
> 1) Change offset and tlv_len to be 32bit - no point trying to do 16bit mathx.
> 2) Worry about non-linear skb - perhaps just process skb_header_len() bytes.
> 3) Allow for very short frames.
> 4) Don't read beyond the end of the skb data.
>
> Something like:
> tlv = skb->data + LLCP_HEADER_SIZE;
> tlv_end = skb_tail_pointer(skb);
> for (; tlv + 2 < tlv_end; tlv += 2 + length) {
> type = tlv[0];
> length = tlv[1];
> if (tlv + 2 + length > tlv_end)
> break;
>
> Then as well as the check for length >= 1 in SDREQ (even 1 doesn't make sense),
> the SDRES path is missing a check for length >= 2.
[ just moved sec@k.o to Bcc since this is public discussion, please
don't re-add it ]
Thanks,
Willy
next prev parent reply other threads:[~2026-06-03 17:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-25 20:24 [PATCH] nfc: llcp: fix integer underflow and missing bounds checks in TLV parsing Doruk Tan Ozturk
2026-06-02 11:12 ` David Heidelberg
2026-06-02 12:58 ` David Laight
2026-06-03 16:55 ` Willy Tarreau [this message]
-- strict thread matches above, loose matches on Subject: below --
2026-05-25 20:21 Doruk Tan Ozturk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aiBcfsXn0BhBi5ae@1wt.eu \
--to=w@1wt.eu \
--cc=david+nfc@ixit.cz \
--cc=david.laight.linux@gmail.com \
--cc=doruk@0sec.ai \
--cc=netdev@vger.kernel.org \
--cc=oe-linux-nfc@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.