Re: [PATCH] nfc: fdp: reject an oversized device-reported packet length

[Bryam Vargas <hexlabsecurity@proton.me>]

> So I wonder if the check you are adding below should also guard
> against phy->next_read_size < FDP_NCI_I2C_MIN_PAYLOAD.

Agreed. Both routes are reachable: a length packet with tmp[2] == 0 &&
tmp[3] < 2 gives 3 or 4 directly, and the + 3 can carry the u16 store
past 65535 (tmp[2] == tmp[3] == 0xff -> 65538, truncated to 2).
next_read_size then drops below the 2-byte header + 1-byte LRC that
fdp_nci_i2c_remove_len_lrc() strips, and the short read also leaves
tmp[2]/tmp[3] unrefreshed when the next packet is parsed as a length
packet, so the following length comes from stale buffer bytes. It is not
a second overflow -- skb_pull()/skb_trim() are self-guarding, so the
frame is malformed rather than out of bounds -- but the desync is real
and bounding it is correct.

v2 folds both ends into one range check on the stored value, so the
truncation case is covered too. It also carries a fix for an skb leak in
the same function (a device that sends two data packets in one call
leaks the first skb); I am happy to split that into its own patch if you
would prefer.

I will add your Suggested-by.

Bryam