For some reason, these two NFC patches fell through the cracks. It
looks like sometimes NFC patches go through the wireless tree, and
other times, through the networking tree directly.
I don't care which way they go, but they should get merged through some
way, or, I can take them in my device tree, but that feels a bit odd to
me.
thanks,
greg k-h
Suren Baghdasaryan (2):
NFC: Fix possible memory corruption when handling SHDLC I-Frame
commands
NFC: Fix the number of pipes
include/net/nfc/hci.h | 2 +-
net/nfc/hci/core.c | 10 ++++++++++
2 files changed, 11 insertions(+), 1 deletion(-)
--
2.19.0
From: Greg Kroah-Hartman <[email protected]>
Date: Mon, 17 Sep 2018 15:51:39 +0200
> For some reason, these two NFC patches fell through the cracks. It
> looks like sometimes NFC patches go through the wireless tree, and
> other times, through the networking tree directly.
>
> I don't care which way they go, but they should get merged through some
> way, or, I can take them in my device tree, but that feels a bit odd to
> me.
I'll take these directly, thanks Greg.
From: Suren Baghdasaryan <[email protected]>
When handling SHDLC I-Frame commands "pipe" field used for indexing
into an array should be checked before usage. If left unchecked it
might access memory outside of the array of size NFC_HCI_MAX_PIPES(127).
Malformed NFC HCI frames could be injected by a malicious NFC device
communicating with the device being attacked (remote attack vector),
or even by an attacker with physical access to the I2C bus such that
they could influence the data transfers on that bus (local attack vector).
skb->data is controlled by the attacker and has only been sanitized in
the most trivial ways (CRC check), therefore we can consider the
create_info struct and all of its members to tainted. 'create_info->pipe'
with max value of 255 (uint8) is used to take an offset of the
hdev->pipes array of 127 elements which can lead to OOB write.
Cc: Samuel Ortiz <[email protected]>
Cc: Allen Pais <[email protected]>
Cc: "David S. Miller" <[email protected]>
Suggested-by: Kevin Deus <[email protected]>
Signed-off-by: Suren Baghdasaryan <[email protected]>
Acked-by: Kees Cook <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
net/nfc/hci/core.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/nfc/hci/core.c b/net/nfc/hci/core.c
index ac8030c4bcf8..19cb2e473ea6 100644
--- a/net/nfc/hci/core.c
+++ b/net/nfc/hci/core.c
@@ -209,6 +209,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
}
create_info = (struct hci_create_pipe_resp *)skb->data;
+ if (create_info->pipe >= NFC_HCI_MAX_PIPES) {
+ status = NFC_HCI_ANY_E_NOK;
+ goto exit;
+ }
+
/* Save the new created pipe and bind with local gate,
* the description for skb->data[3] is destination gate id
* but since we received this cmd from host controller, we
@@ -232,6 +237,11 @@ void nfc_hci_cmd_received(struct nfc_hci_dev *hdev, u8 pipe, u8 cmd,
}
delete_info = (struct hci_delete_pipe_noti *)skb->data;
+ if (delete_info->pipe >= NFC_HCI_MAX_PIPES) {
+ status = NFC_HCI_ANY_E_NOK;
+ goto exit;
+ }
+
hdev->pipes[delete_info->pipe].gate = NFC_HCI_INVALID_GATE;
hdev->pipes[delete_info->pipe].dest_host = NFC_HCI_INVALID_HOST;
break;
--
2.19.0
From: Suren Baghdasaryan <[email protected]>
According to ETSI TS 102 622 specification chapter 4.4 pipe identifier
is 7 bits long which allows for 128 unique pipe IDs. Because
NFC_HCI_MAX_PIPES is used as the number of pipes supported and not
as the max pipe ID, its value should be 128 instead of 127.
nfc_hci_recv_from_llc extracts pipe ID from packet header using
NFC_HCI_FRAGMENT(0x7F) mask which allows for pipe ID value of 127.
Same happens when NCI_HCP_MSG_GET_PIPE() is being used. With
pipes array having only 127 elements and pipe ID of 127 the OOB memory
access will result.
Cc: Samuel Ortiz <[email protected]>
Cc: Allen Pais <[email protected]>
Cc: "David S. Miller" <[email protected]>
Suggested-by: Dan Carpenter <[email protected]>
Signed-off-by: Suren Baghdasaryan <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Cc: stable <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
include/net/nfc/hci.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/nfc/hci.h b/include/net/nfc/hci.h
index 316694dafa5b..008f466d1da7 100644
--- a/include/net/nfc/hci.h
+++ b/include/net/nfc/hci.h
@@ -87,7 +87,7 @@ struct nfc_hci_pipe {
* According to specification 102 622 chapter 4.4 Pipes,
* the pipe identifier is 7 bits long.
*/
-#define NFC_HCI_MAX_PIPES 127
+#define NFC_HCI_MAX_PIPES 128
struct nfc_hci_init_data {
u8 gate_count;
struct nfc_hci_gate gates[NFC_HCI_MAX_CUSTOM_GATES];
--
2.19.0