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

From: Johan Hovold

Date: Tue Sep 08 2026 - 06:12:28 EST


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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=1853daab1a47603d4678
> Tested-by: syzbot+1853daab1a47603d4678@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Deepanshu Kartikey <kartikey406@xxxxxxxxx>

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