Skip to contentSkip to conversation

NFC: prevent multiple buffer overflows in NCI

PATCH In review Dan Rosenberg 2 participants 2 messages Jun 21, 2012 – Jun 24, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 NFC: prevent multiple buffer overflows in NCI

Fix multiple remotely-exploitable stack-based buffer overflows due to the NCI code pulling length fields directly from incoming frames and copying too much data into statically-sized arrays. Fortunately, there don't appear to be any active users of this code (yet).

This patch fixes the overflows, but I suspect the code will need to be completely reworked since this doesn't address the more systemic problem of failing to check that the values read from incoming frame data aren't from beyond the end of the pulled skb data. Build tested only.

Cc Lauro Ramos Venancio <[email protected]>
Cc Aloisio Almeida Jr <[email protected]>
Cc Samuel Ortiz <[email protected]>
Cc David S. Miller <[email protected]>
Cc Ilan Elias <[email protected]>
---
 net/nfc/nci/ntf.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)
net/nfc/nci/ntf.c +0 -0
index cb26461..2ab196a 100644
@@ -106,7 +106,7 @@ static __u8 *nci_extract_rf_params_nfca_passive_poll(struct

nci_dev *ndev,
nfca_poll->sens_res = __le16_to_cpu(*((__u16 *)data));
data += 2;
- nfca_poll->nfcid1_len = *data++;
+ nfca_poll->nfcid1_len = min_t(__u8, *data++, NFC_NFCID1_MAXSIZE);
pr_debug("sens_res 0x%x, nfcid1_len %d\n",
nfca_poll->sens_res, nfca_poll->nfcid1_len);
@@ -130,7 +130,7 @@ static __u8 *nci_extract_rf_params_nfcb_passive_poll(struct nci_dev *ndev,
struct rf_tech_specific_params_nfcb_poll *nfcb_poll,
__u8 *data)
{
- nfcb_poll->sensb_res_len = *data++;
+ nfcb_poll->sensb_res_len = min_t(__u8, *data++, NFC_SENSB_RES_MAXSIZE);
pr_debug("sensb_res_len %d\n", nfcb_poll->sensb_res_len);
@@ -145,7 +145,7 @@ static __u8 *nci_extract_rf_params_nfcf_passive_poll(struct
nci_dev *ndev,
__u8 *data)
{
nfcf_poll->bit_rate = *data++;
- nfcf_poll->sensf_res_len = *data++;
+ nfcf_poll->sensf_res_len = min_t(__u8, *data++, NFC_SENSF_RES_MAXSIZE);
pr_debug("bit_rate %d, sensf_res_len %d\n",
nfcf_poll->bit_rate, nfcf_poll->sensf_res_len);
@@ -331,7 +331,7 @@ static int nci_extract_activation_params_iso_dep(struct nci_dev *ndev,
switch (ntf->activation_rf_tech_and_mode) {
case NCI_NFC_A_PASSIVE_POLL_MODE:
nfca_poll = &ntf->activation_params.nfca_poll_iso_dep;
- nfca_poll->rats_res_len = *data++;
+ nfca_poll->rats_res_len = min_t(__u8, *data++, 20);
pr_debug("rats_res_len %d\n", nfca_poll->rats_res_len); if (nfca_poll->rats_res_len > 0) {
memcpy(nfca_poll->rats_res,
@@ -341,7 +341,7 @@ static int nci_extract_activation_params_iso_dep(struct nci_dev *ndev,
case NCI_NFC_B_PASSIVE_POLL_MODE:
nfcb_poll = &ntf->activation_params.nfcb_poll_iso_dep;
- nfcb_poll->attrib_res_len = *data++;
+ nfcb_poll->attrib_res_len = min_t(__u8, *data++, 50);
pr_debug("attrib_res_len %d\n", nfcb_poll->attrib_res_len); if (nfcb_poll->attrib_res_len > 0) {
memcpy(nfcb_poll->attrib_res,

Acked-by on v1 1/1
#1 lore raw

Hi Dan,

Quoted 24 lines

From: Dan Rosenberg [mailto:[email protected]]
Sent: Thursday, June 21, 2012 10:56 PM
To: [email protected]; [email protected]; [email protected]; David Miller; Elias, Ilan
Cc: [email protected]; [email protected]; [email protected]
Subject: [PATCH] NFC: prevent multiple buffer overflows in NCI

Fix multiple remotely-exploitable stack-based buffer overflows due to the NCI
code pulling length fields directly from incoming frames and copying too much
data into statically-sized arrays. Fortunately, there don't appear to be any
active users of this code (yet).

This patch fixes the overflows, but I suspect the code will need to be completely reworked since this doesn't address the more systemic problem of
failing to check that the values read from incoming frame data aren't from
beyond the end of the pulled skb data. Build tested only.

Quoted 8 lines
Cc Lauro Ramos Venancio <[email protected]>
Cc Aloisio Almeida Jr <[email protected]>
Cc Samuel Ortiz <[email protected]>
Cc David S. Miller <[email protected]>
Cc Ilan Elias <[email protected]>

Thanks & BR,
Ilan