[PATCH net v3 1/4] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep

[PATCH net v3 1/4] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep

[Lekë Hapçiu]

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

nci_store_general_bytes_nfc_dep() computes the General Bytes length by
subtracting a fixed header offset from the peer-supplied atr_res_len
(POLL) or atr_req_len (LISTEN) field:

    ndev->remote_gb_len = min_t(__u8,
        atr_res_len - NFC_ATR_RES_GT_OFFSET,   /* offset = 15 */
        NFC_ATR_RES_GB_MAXSIZE);

Both length fields are __u8.  When a malicious NFC-DEP peer sends an
ATR_RES/ATR_REQ whose length is smaller than the fixed offset (< 15
or < 14 respectively), the subtraction wraps:

    atr_res_len = 0  ->  (u8)(0 - 15) = 241
    min_t(__u8, 241, NFC_ATR_RES_GB_MAXSIZE=47) = 47

The subsequent memcpy then reads 47 bytes beyond the valid activation
parameter data into ndev->remote_gb[].  This buffer is later fed to
nfc_llcp_parse_gb_tlv() as a TLV array.

Reject the frame with NCI_STATUS_RF_PROTOCOL_ERROR when the length is
below the required offset, and propagate the error out of
nci_rf_intf_activated_ntf_packet() instead of silently accepting the
malformed packet.

Reachable from any NFC peer within ~4 cm during RF activation, prior
to any pairing.

Fixes: c4fbb6515709 ("NFC: NCI: Add NFC-DEP support to NCI data exchange")
Cc: stable@vger.kernel.org
Signed-off-by: Lekë Hapçiu <framemain@outlook.com>
---
 net/nfc/nci/ntf.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/nfc/nci/ntf.c b/net/nfc/nci/ntf.c
index c96512bb8..eb8c6e5a1 100644
--- a/net/nfc/nci/ntf.c
+++ b/net/nfc/nci/ntf.c
@@ -631,6 +631,9 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev,
 	switch (ntf->activation_rf_tech_and_mode) {
 	case NCI_NFC_A_PASSIVE_POLL_MODE:
 	case NCI_NFC_F_PASSIVE_POLL_MODE:
+		if (ntf->activation_params.poll_nfc_dep.atr_res_len <
+		    NFC_ATR_RES_GT_OFFSET)
+			return NCI_STATUS_RF_PROTOCOL_ERROR;
 		ndev->remote_gb_len = min_t(__u8,
 			(ntf->activation_params.poll_nfc_dep.atr_res_len
 						- NFC_ATR_RES_GT_OFFSET),
@@ -643,6 +646,9 @@ static int nci_store_general_bytes_nfc_dep(struct nci_dev *ndev,
 
 	case NCI_NFC_A_PASSIVE_LISTEN_MODE:
 	case NCI_NFC_F_PASSIVE_LISTEN_MODE:
+		if (ntf->activation_params.listen_nfc_dep.atr_req_len <
+		    NFC_ATR_REQ_GT_OFFSET)
+			return NCI_STATUS_RF_PROTOCOL_ERROR;
 		ndev->remote_gb_len = min_t(__u8,
 			(ntf->activation_params.listen_nfc_dep.atr_req_len
 						- NFC_ATR_REQ_GT_OFFSET),
@@ -842,8 +848,10 @@ static int nci_rf_intf_activated_ntf_packet(struct nci_dev *ndev,
 		/* store general bytes to be reported later in dep_link_up */
 		if (ntf.rf_interface == NCI_RF_INTERFACE_NFC_DEP) {
 			err = nci_store_general_bytes_nfc_dep(ndev, &ntf);
-			if (err != NCI_STATUS_OK)
+			if (err != NCI_STATUS_OK) {
 				pr_err("unable to store general bytes\n");
+				return -EINVAL;
+			}
 		}
 
 		/* store ATS to be reported later in nci_activate_target */
-- 
2.51.0

Re: [PATCH net v3 1/4] nfc: nci: fix u8 underflow in nci_store_general_bytes_nfc_dep

[Simon Horman]

On Wed, Apr 15, 2026 at 01:35:30AM +0200, Lekë Hapçiu wrote:
> From: Lekë Hapçiu <framemain@outlook.com>
> 
> nci_store_general_bytes_nfc_dep() computes the General Bytes length by
> subtracting a fixed header offset from the peer-supplied atr_res_len
> (POLL) or atr_req_len (LISTEN) field:
> 
>     ndev->remote_gb_len = min_t(__u8,
>         atr_res_len - NFC_ATR_RES_GT_OFFSET,   /* offset = 15 */
>         NFC_ATR_RES_GB_MAXSIZE);
> 
> Both length fields are __u8.  When a malicious NFC-DEP peer sends an
> ATR_RES/ATR_REQ whose length is smaller than the fixed offset (< 15
> or < 14 respectively), the subtraction wraps:
> 
>     atr_res_len = 0  ->  (u8)(0 - 15) = 241
>     min_t(__u8, 241, NFC_ATR_RES_GB_MAXSIZE=47) = 47
> 
> The subsequent memcpy then reads 47 bytes beyond the valid activation
> parameter data into ndev->remote_gb[].  This buffer is later fed to
> nfc_llcp_parse_gb_tlv() as a TLV array.
> 
> Reject the frame with NCI_STATUS_RF_PROTOCOL_ERROR when the length is
> below the required offset, and propagate the error out of
> nci_rf_intf_activated_ntf_packet() instead of silently accepting the
> malformed packet.

This does not seem to be consistent with the handling of other in
nci_rf_intf_activated_ntf_packet() when it calls other functions similar to
nci_rf_intf_activated_ntf_packet().

I suggest dropping this part of the fix, and addressing
nci_rf_intf_activated_ntf_packet() in a more holistic manner
if this kind of change is desired.

> 
> Reachable from any NFC peer within ~4 cm during RF activation, prior
> to any pairing.

I do not understand how this statement relates to this change.
Could you explain?

> 
> Fixes: c4fbb6515709 ("NFC: NCI: Add NFC-DEP support to NCI data exchange")

I am unable to find a commit with either that hash or subject.

It seems to me that this problem was introduced in:

767f19ae698e ("NFC: Implement NCI dep_link_up and dep_link_down")

-- 
pw-bot: changes-requested