Re: [PATCH] nfc: llcp: fix integer underflow and missing bounds checks in TLV parsing
David Laight <[email protected]> Tue, 2 Jun 2026 13:58:55 +0100
| Newsgroups | dev.linux.lists.oe-linux-nfc,org.kernel.vger.netdev |
|---|---|
| Message-ID | <20260602135610.5f79ed10@pumpkin> |
On Mon, 25 May 2026 22:24:27 +0200 Doruk Tan Ozturk <[email protected]> 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. -- David > > Found by pwnkit (https://github.com/0sec-labs/pwnkit), an automated > kernel source review tool by 0sec (https://0sec.ai). > > Signed-off-by: Doruk Tan Ozturk <[email protected]> > --- > net/nfc/llcp_core.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/net/nfc/llcp_core.c b/net/nfc/llcp_core.c > index XXXXXXX..YYYYYYY 100644 > --- a/net/nfc/llcp_core.c > +++ b/net/nfc/llcp_core.c > @@ -1300,6 +1300,9 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local, > > while (offset < tlv_len) { > + if (offset + 2 > tlv_len) > + break; > + > type = tlv[0]; > length = tlv[1]; > > @@ -1307,6 +1310,9 @@ static void nfc_llcp_recv_snl(struct nfc_llcp_local *local, > switch (type) { > case LLCP_TLV_SDREQ: > + if (length < 1) > + break; > + > tid = tlv[2]; > service_name = (char *) &tlv[3]; > service_name_len = length - 1; >