linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: ST ST95HF DRIVER security bug
       [not found] <CAAMXCFnzLX-yWKSJ5JoBxcE8E0=cSQeDExGoFBxhkusUNeYncg@mail.gmail.com>
@ 2022-08-24 12:08 ` Krzysztof Kozlowski
       [not found]   ` <CAAMXCF=15tSmz7=nzVRgw166wDmMGiBuLx6Of-NLvboMN3nAuQ@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-24 12:08 UTC (permalink / raw)
  To: מיכאל
	שטראוס,
	netdev, linux-kernel
  Cc: Jakub Kicinski, David S. Miller, linux-nfc

On 24/08/2022 12:10, מיכאל שטראוס wrote:
> Hi,
> I found a small security bug in the ST95HF driver in Linux kernel .
> Thought it is responsible to report it to you guys so you we can patch it
> up.

Please use scripts/get_maintainers.pl to Cc relevant people. You got the
same comment last time as well...

> CVE ID was requested,  when it is reserved I will update.
> 
> ST ST95HF DRIVER
> ST95HF is an integrated transceiver for NFC made by ST,
> Buffer overflow can be triggered by the attacker by providing malicious
> size in one of the SPI receive registers.
> 
> *Details:*
> ```jsx
> unsigned char st95hf_response_arr[2];
> ret = st95hf_spi_recv_response(&st95context->spicontext,
>       st95hf_response_arr);
> ...
> 
> /* Support of long frame */
> if (receivebuff[0] & 0x60)
> len += (((receivebuff[0] & 0x60) >> 5) << 8) | receivebuff[1];
> else
> len += receivebuff[1];
> 
> /* Now make a transfer to read only relevant bytes */
> tx_takedata.rx_buf = &receivebuff[2];
> tx_takedata.len = len - 2;
> 
> spi_message_init(&m);
> spi_message_add_tail(&tx_takedata, &m);
> ```
> Driver sets a buffer of 2 bytes for the input bytes but actually allows the
> driver to overflow it with any valid SPI message (short or long frame) in
> the tx_takedata stage.
> It seems like a mistake, but i may be missing something and i am totally
> wrong.
> 
> *Effected commits:* Current source includes issue.

What does it mean "current source"? Please be specific which exactly
kernel version is affected, which commit introduced it.

> *Exploitable:* I actually think vulnerability can be exploitable by any
> device on the SPI bus.

Then the risk is quite low, right? SPI busses are not user hot-pluggable
except some development boards (so again a real niche). Basically it's
impact is negligible, considering that system would need to have such
device reported and configured (which for DT systems is static) and at
the same time malfunctioning or behaving incorrectly?

You already sent me a message about it in May and this does not bring
anything particularly new since then. You did not respond to my reply
that time, although maybe that's continuation.

Anyway proper analyzing of this issue and a patch would be nice.

> *Code:*
> <https://github.com/torvalds/linux/blame/master/drivers/nfc/st95hf/core.c#L284>
> https://github.com/torvalds/linux/blame/master/drivers/nfc/st95hf/core.c#L284
> ,
> https://github.com/torvalds/linux/blob/master/drivers/nfc/st95hf/spi.c#L107
> 
> I was not able to understand if the remote device has ability to trigger
> the issue or only the SPI connected device?


What does it mean "remote device"? NFC? NFC tag does not talk over SPI...

Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: ST ST95HF DRIVER security bug
       [not found]   ` <CAAMXCF=15tSmz7=nzVRgw166wDmMGiBuLx6Of-NLvboMN3nAuQ@mail.gmail.com>
@ 2022-08-24 16:21     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2022-08-24 16:21 UTC (permalink / raw)
  To: מיכאל
	שטראוס
  Cc: netdev, linux-kernel, Jakub Kicinski, David S. Miller, linux-nfc

On 24/08/2022 18:12, מיכאל שטראוס wrote:
>>
>> Please use scripts/get_maintainers.pl to Cc relevant people. You got the
>> same comment last time as well...
>>
> Sorry my bad, i forgot we already contacted.
> I actually ran it and your name came up for some reason.
> 
>> ./scripts/get_maintainer.pl drivers/nfc/st95hf/spi.c
> 
> Bad divisor in main::vcs_assign: 0
> 
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> (maintainer:NFC
>> SUBSYSTEM)
> 
> netdev@vger.kernel.org (open list:NFC SUB

and other addresses... why removing them?

> 
> 
> 
> 
>>  What does it mean "current source"? Please be specific which exactly
> 
> kernel version is affected, which commit introduced it.
> 
> *Effected version: *
> - v6.0-rc2 <https://github.com/torvalds/linux/releases/tag/v6.0-rc2>  ...
> - *v4.5-rc1* <https://github.com/torvalds/linux/releases/tag/v4.5-rc1>
> *Introducing commit:  *
> https://github.com/torvalds/linux/commit/cab47333f0f75b685bce1facecb73bf3632e1360
> 
> Then the risk is quite low, right? SPI busses are not user hot-pluggable
>> except some development boards (so again a real niche). Basically it's
>> impact is negligible
>>
> Agreed.
> 
> What does it mean "remote device"? NFC? NFC tag does not talk over SPI...
>>
> I was wondering maybe the tag is the source for the content that actually
> overflows the kernel buffer,
> In which case it changes the picture a bit.

The buffer is used for SPI transfer, so the NFC tag - except that it
works with that device - is rather long shot.


Best regards,
Krzysztof

^ permalink raw reply	[flat|nested] 3+ messages in thread

* ST ST95HF DRIVER security bug
@ 2022-08-25  7:11 מיכאל שטראוס
  0 siblings, 0 replies; 3+ messages in thread
From: מיכאל שטראוס @ 2022-08-25  7:11 UTC (permalink / raw)
  To: linux-kernel, netdev

Hi,
I found a small security bug in the ST95HF driver in Linux kernel .
Thought it is responsible to report it to you guys so you we can patch it up.
CVE ID was requested,  when it is reserved I will update.
I want to thank Krzysztof Kozlowski for helping me out.

ST ST95HF DRIVER
ST95HF is an integrated transceiver for NFC made by ST,
Buffer overflow can be triggered by the attacker by providing
malicious size in one of the SPI receive registers.

Details:
```jsx
unsigned char st95hf_response_arr[2];
ret = st95hf_spi_recv_response(&st95context->spicontext,
      st95hf_response_arr);
...

/* Support of long frame */
if (receivebuff[0] & 0x60)
len += (((receivebuff[0] & 0x60) >> 5) << 8) | receivebuff[1];
else
len += receivebuff[1];

/* Now make a transfer to read only relevant bytes */
tx_takedata.rx_buf = &receivebuff[2];
tx_takedata.len = len - 2;

spi_message_init(&m);
spi_message_add_tail(&tx_takedata, &m);
```
Driver sets a buffer of 2 bytes for the input bytes but actually
allows the driver to overflow it with any valid SPI message (short or
long frame) in the tx_takedata stage.
It seems like a mistake, but i may be missing something and i am totally wrong.

* Exploitable: I actually think vulnerability can be exploitable by
any device on the SPI bus.
   Exploit is quite low risk and impact is negligible, both because it
requires an actual connected device and because it requires the driver
to be registered.

* Effected versions: v6.0-rc2 - v4.5-rc1

* Introducing commit:
https://github.com/torvalds/linux/commit/cab47333f0f75b685bce1facecb73bf3632e1360

* Code:
https://github.com/torvalds/linux/blame/master/drivers/nfc/st95hf/core.c#L284
https://github.com/torvalds/linux/blob/master/drivers/nfc/st95hf/spi.c#L107

Side note:
I was wondering if maybe the tag is the source for the content that actually
overflows the kernel buffer,  In which case it changes the picture a bit.
But if i understood correctly the messages are used for configuration
with connected device.

Suggested patch:
I am actually not sure if we can somehow limit the size to MAX command length.
I changed it to 1024 because it seems to support 10 bit long frame spi.

```
diff --git a/drivers/nfc/st95hf/core.c b/drivers/nfc/st95hf/core.c
index ed704bb77..741ce633b 100644
--- a/drivers/nfc/st95hf/core.c
+++ b/drivers/nfc/st95hf/core.c
@@ -281,7 +281,7 @@ static int st95hf_send_recv_cmd(struct
st95hf_context *st95context,
        }

        if (cmd_array[cmd].req == SYNC && recv_res) {
-               unsigned char st95hf_response_arr[2];
+               unsigned char st95hf_response_arr[1024];

                ret = st95hf_spi_recv_response(&st95context->spicontext,
                                               st95hf_response_arr);
```

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-08-25  7:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAAMXCFnzLX-yWKSJ5JoBxcE8E0=cSQeDExGoFBxhkusUNeYncg@mail.gmail.com>
2022-08-24 12:08 ` ST ST95HF DRIVER security bug Krzysztof Kozlowski
     [not found]   ` <CAAMXCF=15tSmz7=nzVRgw166wDmMGiBuLx6Of-NLvboMN3nAuQ@mail.gmail.com>
2022-08-24 16:21     ` Krzysztof Kozlowski
2022-08-25  7:11 מיכאל שטראוס

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).