Search Linux Wireless

Re: NFC: st21nfca: Add HCI transaction event support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

Actually, i am sorry, i forgot to reply back on your email but i have tried to add comments in a follow up patch for st21nfca and st21nfcb:
st21nfca: https://lists.01.org/pipermail/linux-nfc/2015-March/003463.html
st21nfcb: https://lists.01.org/pipermail/linux-nfc/2015-March/003462.html

The actual answer from an nfc evt_transaction is a known structure with secure element application identifier (aid) + some data.

Please let me know if it is still not clear enough.

Best Regards

On 05/06/2015 12:59, Dan Carpenter wrote:
I never got a response on this.  Is this remote exploitable or from the
firmware?

regards,
dan carpenter

On Tue, Feb 10, 2015 at 12:00:51PM +0300, Dan Carpenter wrote:
Hello Christophe Ricard,

The patch 26fc6c7f02cb: "NFC: st21nfca: Add HCI transaction event
support" from Feb 1, 2015, leads to the following static checker
warning:

	drivers/nfc/st21nfca/st21nfca_se.c:321 st21nfca_connectivity_event_received()
	error: 'skb->data[1]' from user is not capped properly

drivers/nfc/st21nfca/st21nfca_se.c
    300  int st21nfca_connectivity_event_received(struct nfc_hci_dev *hdev, u8 host,
    301                                  u8 event, struct sk_buff *skb)
    302  {
    303          int r = 0;
    304          struct device *dev = &hdev->ndev->dev;
    305          struct nfc_evt_transaction *transaction;
    306
    307          pr_debug("connectivity gate event: %x\n", event);
    308
    309          switch (event) {
    310          case ST21NFCA_EVT_CONNECTIVITY:
    311                  break;
    312          case ST21NFCA_EVT_TRANSACTION:
    313                  if (skb->len < NFC_MIN_AID_LENGTH + 2 &&
    314                      skb->data[0] != NFC_EVT_TRANSACTION_AID_TAG)
    315                          return -EPROTO;

Here we don't trust skb->data[0].

    316
    317                  transaction = (struct nfc_evt_transaction *)devm_kzalloc(dev,
    318                                                     skb->len - 2, GFP_KERNEL);
    319
    320                  transaction->aid_len = skb->data[1];
    321                  memcpy(transaction->aid, &skb->data[2], skb->data[1]);
                                                                 ^^^^^^^^^^^^
But here we trust skb->data[1].

NFC code is hard to analyze because sometimes skb->data[] comes from the
firmware and holds trusted values.  But sometimes it comes from the
network and can overflow.  Smatch marks it all as untrusted so it causes
a lot of false postives.

Some of them have comments like:

	net/nfc/hci/core.c:218 nfc_hci_cmd_received()
	error: buffer overflow 'hdev->pipes' 127 <= 255

But this one doesn't have a comment so it's hard for me as an outsider
to say if this is a bug or not.

    322

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux