Netdev List
 help / color / mirror / Atom feed
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

  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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox