[v1] nfc: pn533: fix OOB read in pn533_acr122_is_rx_frame_valid()

[PATCH] nfc: pn533: fix OOB read in pn533_acr122_is_rx_frame_valid()

Posted by Deepanshu Kartikey 2 weeks, 5 days ago

frame->ccid.datalen is read directly from the USB response frame
and used, unchecked, as an index into frame->data[]. A malicious or
malfunctioning device can set this field to an arbitrary value,
causing the driver to read far outside the received buffer.

Bound ccid.datalen against the maximum possible ACR122 frame size
before using it, and reject values less than 2 to avoid the
"datalen - 2" underflowing.

Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
Reported-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1853daab1a47603d4678
Tested-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 drivers/nfc/pn533/usb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index efb07f944fce..d08d3b0df988 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -322,6 +322,11 @@ static bool pn533_acr122_is_rx_frame_valid(void *_frame, struct pn533 *dev)
 	if (!frame->ccid.datalen)
 		return false;
 
+	if (frame->ccid.datalen < 2 ||
+	    frame->ccid.datalen > PN533_ACR122_FRAME_MAX_PAYLOAD_LEN +
+	    PN533_ACR122_RX_FRAME_TAIL_LEN)
+		return false;
+
 	if (frame->data[frame->ccid.datalen - 2] == 0x63)
 		return false;
 
-- 
2.43.0

Re: [PATCH] nfc: pn533: fix OOB read in pn533_acr122_is_rx_frame_valid()

Posted by Johan Hovold 2 weeks, 3 days ago

On Sun, Sep 06, 2026 at 11:13:36AM +0530, Deepanshu Kartikey wrote:
> frame->ccid.datalen is read directly from the USB response frame
> and used, unchecked, as an index into frame->data[]. A malicious or
> malfunctioning device can set this field to an arbitrary value,
> causing the driver to read far outside the received buffer.
> 
> Bound ccid.datalen against the maximum possible ACR122 frame size
> before using it, and reject values less than 2 to avoid the
> "datalen - 2" underflowing.
> 
> Fixes: 9815c7cf22da ("NFC: pn533: Separate physical layer from the core implementation")
> Reported-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1853daab1a47603d4678
> Tested-by: syzbot+1853daab1a47603d4678@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Are you missing an Assisted-by tag here?

> ---
>  drivers/nfc/pn533/usb.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
> index efb07f944fce..d08d3b0df988 100644
> --- a/drivers/nfc/pn533/usb.c
> +++ b/drivers/nfc/pn533/usb.c
> @@ -322,6 +322,11 @@ static bool pn533_acr122_is_rx_frame_valid(void *_frame, struct pn533 *dev)
>  	if (!frame->ccid.datalen)
>  		return false;
>  
> +	if (frame->ccid.datalen < 2 ||
> +	    frame->ccid.datalen > PN533_ACR122_FRAME_MAX_PAYLOAD_LEN +
> +	    PN533_ACR122_RX_FRAME_TAIL_LEN)
> +		return false;
> +

Just a drive-by comment, but should this not replace the existing
datalen == 0 check?

>  	if (frame->data[frame->ccid.datalen - 2] == 0x63)
>  		return false;

Johan

Re: [PATCH] nfc: pn533: fix OOB read in pn533_acr122_is_rx_frame_valid()

Posted by Deepanshu Kartikey 2 weeks, 2 days ago

On Tue, Sep 8, 2026 at 2:48 PM Johan Hovold <johan@kernel.org> wrote:
>

> Are you missing an Assisted-by tag here?
>

Sure , I will add assisted-by tag .

> Just a drive-by comment, but should this not replace the existing
> datalen == 0 check?
>
> >       if (frame->data[frame->ccid.datalen - 2] == 0x63)
> >               return false;
>
> Johan

I will remove the older if (!frame->ccid.datalen)   check.

I will send patch v2 shortly

Thanks

Deepanshu