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@xxxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Lekë Hapçiu <framemain@xxxxxxxxxxx>

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.

...