[PATCH net v3 2/4] nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv

[PATCH net v3 2/4] nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv

[Lekë Hapçiu]

From: Lekë Hapçiu <framemain@outlook.com>

nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() walk TLV
arrays whose length and content come from a peer-supplied frame.  The
parsing loop has three weaknesses:

 1. `offset` is declared u8 while `tlv_array_len` is u16.  In
    parse_connection_tlv() the TLV array can reach ~2173 bytes (MIUX
    up to 0x7FF), so 128 zero-length TLVs wrap `offset` back to 0 and
    the loop never terminates while `tlv` advances past the buffer.

 2. The guard `offset < tlv_array_len` only proves one byte is
    available, but the body reads tlv[0] (type) and tlv[1] (length).
    When one byte remains, tlv[1] is out of bounds.

 3. `length` is read from peer data and used to advance `tlv` without
    being checked against the remaining array space.  A crafted length
    walks `tlv` past the buffer; the next iteration reads tlv[0]/tlv[1]
    from adjacent memory.

The llcp_tlv8() and llcp_tlv16() accessors additionally read tlv[2]
and tlv[2..3]; a zero-length TLV makes those reads out of bounds.

Fix: promote `offset` to u16; add two per-iteration guards, one for
the TLV header and one for the TLV value; require length >= 1 for all
TLVs before the type dispatch and length >= 2 for the llcp_tlv16()
accessors (MIUX, WKS).  Return -EINVAL on malformed input.

Reached on ATR_RES (parse_gb_tlv) and on CONNECT/CC PDUs before a
connection is established (parse_connection_tlv).  Both are
triggerable from any NFC peer within ~4 cm, without authentication.

Reported-by: Simon Horman <horms@kernel.org>
Fixes: d646960f7986 ("NFC: Add LLCP sockets")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/llcp_commands.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/net/nfc/llcp_commands.c b/net/nfc/llcp_commands.c
index 291f26fac..b6dcfb2d1 100644
--- a/net/nfc/llcp_commands.c
+++ b/net/nfc/llcp_commands.c
@@ -193,7 +193,8 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 			  const u8 *tlv_array, u16 tlv_array_len)
 {
 	const u8 *tlv = tlv_array;
-	u8 type, length, offset = 0;
+	u8 type, length;
+	u16 offset = 0;
 
 	pr_debug("TLV array length %d\n", tlv_array_len);
 
@@ -201,8 +202,14 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 		return -ENODEV;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
 		type = tlv[0];
 		length = tlv[1];
+		if (tlv_array_len - offset - 2 < length)
+			return -EINVAL;
+		if (length < 1)
+			return -EINVAL;
 
 		pr_debug("type 0x%x length %d\n", type, length);
 
@@ -211,9 +218,13 @@ int nfc_llcp_parse_gb_tlv(struct nfc_llcp_local *local,
 			local->remote_version = llcp_tlv_version(tlv);
 			break;
 		case LLCP_TLV_MIUX:
+			if (length < 2)
+				return -EINVAL;
 			local->remote_miu = llcp_tlv_miux(tlv) + 128;
 			break;
 		case LLCP_TLV_WKS:
+			if (length < 2)
+				return -EINVAL;
 			local->remote_wks = llcp_tlv_wks(tlv);
 			break;
 		case LLCP_TLV_LTO:
@@ -243,7 +254,8 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 				  const u8 *tlv_array, u16 tlv_array_len)
 {
 	const u8 *tlv = tlv_array;
-	u8 type, length, offset = 0;
+	u8 type, length;
+	u16 offset = 0;
 
 	pr_debug("TLV array length %d\n", tlv_array_len);
 
@@ -251,13 +263,21 @@ int nfc_llcp_parse_connection_tlv(struct nfc_llcp_sock *sock,
 		return -ENOTCONN;
 
 	while (offset < tlv_array_len) {
+		if (tlv_array_len - offset < 2)
+			return -EINVAL;
 		type = tlv[0];
 		length = tlv[1];
+		if (tlv_array_len - offset - 2 < length)
+			return -EINVAL;
+		if (length < 1)
+			return -EINVAL;
 
 		pr_debug("type 0x%x length %d\n", type, length);
 
 		switch (type) {
 		case LLCP_TLV_MIUX:
+			if (length < 2)
+				return -EINVAL;
 			sock->remote_miu = llcp_tlv_miux(tlv) + 128;
 			break;
 		case LLCP_TLV_RW:
-- 
2.51.0

Re: [PATCH net v3 2/4] nfc: llcp: fix TLV parsing in parse_gb_tlv and parse_connection_tlv

[Simon Horman]

On Wed, Apr 15, 2026 at 01:35:31AM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nfc_llcp_parse_gb_tlv() and nfc_llcp_parse_connection_tlv() walk TLV
> arrays whose length and content come from a peer-supplied frame.  The
> parsing loop has three weaknesses:
> 
>  1. `offset` is declared u8 while `tlv_array_len` is u16.  In
>     parse_connection_tlv() the TLV array can reach ~2173 bytes (MIUX
>     up to 0x7FF), so 128 zero-length TLVs wrap `offset` back to 0 and
>     the loop never terminates while `tlv` advances past the buffer.
> 
>  2. The guard `offset < tlv_array_len` only proves one byte is
>     available, but the body reads tlv[0] (type) and tlv[1] (length).
>     When one byte remains, tlv[1] is out of bounds.
> 
>  3. `length` is read from peer data and used to advance `tlv` without
>     being checked against the remaining array space.  A crafted length
>     walks `tlv` past the buffer; the next iteration reads tlv[0]/tlv[1]
>     from adjacent memory.
> 
> The llcp_tlv8() and llcp_tlv16() accessors additionally read tlv[2]
> and tlv[2..3]; a zero-length TLV makes those reads out of bounds.
> 
> Fix: promote `offset` to u16; add two per-iteration guards, one for
> the TLV header and one for the TLV value; require length >= 1 for all
> TLVs before the type dispatch and length >= 2 for the llcp_tlv16()
> accessors (MIUX, WKS).  Return -EINVAL on malformed input.
> 
> Reached on ATR_RES (parse_gb_tlv) and on CONNECT/CC PDUs before a
> connection is established (parse_connection_tlv).  Both are
> triggerable from any NFC peer within ~4 cm, without authentication.

As per my comment on patch 1/4, I don't understand the relationship
between the last sentence above and this patch.

> 
> Reported-by: Simon Horman <horms@kernel.org>
> Fixes: d646960f7986 ("NFC: Add LLCP sockets")

I think the hash but not the subject is correct in the fixes line.
IOW, I think this should be:

Fixes: d646960f7986 ("NFC: Initial LLCP support")

> Cc: stable@vger.kernel.org
> Signed-off-by: Lekë Hapçiu <framemain@outlook.com>

Otherwise, looks good to me.


While looking over this I noticed that nfc_llcp_connect_sn() seems
to have the same kind of problem. You may wish to address that as
a follow-up.

...