On 18.02.2025 23:32:28, Vincent Mailhol wrote:
> Commit 7fdaf8966aae ("can: ucan: use strscpy() to instead of strncpy()")
> unintentionally introduced a one byte out of bound read on strscpy()'s
> source argument (which is kind of ironic knowing that strscpy() is meant
> to be a more secure alternative :)).
>
> Let's consider below buffers:
>
> dest[len + 1]; /* will be NUL terminated */
> src[len]; /* may not be NUL terminated */
>
> When doing:
>
> strncpy(dest, src, len);
> dest[len] = '\0';
>
> strncpy() will read up to len bytes from src.
>
> On the other hand:
>
> strscpy(dest, src, len + 1);
>
> will read up to len + 1 bytes from src, that is to say, an out of bound
> read of one byte will occur on src if it is not NUL terminated. Note
> that the src[len] byte is never copied, but strscpy() still needs to
> read it to check whether a truncation occurred or not.
>
> This exact pattern happened in ucan.
>
> The root cause is that the source is not NUL terminated. Instead of
> doing a copy in a local buffer, directly NUL terminate it as soon as
> usb_control_msg() returns. With this, the local firmware_str[] variable
> can be removed.
>
> On top of this do a couple refactors:
>
> - ucan_ctl_payload->raw is only used for the firmware string, so
> rename it to ucan_ctl_payload->fw_str and change its type from u8 to
> char.
>
> - ucan_device_request_in() is only used to retrieve the firmware
> string, so rename it to ucan_get_fw_str() and refactor it to make it
> directly handle all the string termination logic.
>
> Reported-by: syzbot+d7d8c418e8317899e88c@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/linux-can/67b323a4.050a0220.173698.002b.GAE@xxxxxxxxxx/
> Fixes: 7fdaf8966aae ("can: ucan: use strscpy() to instead of strncpy()")
> Signed-off-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>
Wow! Thanks for the detailed analysis and description of the problem!
Applied to linux-can.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
Attachment:
signature.asc
Description: PGP signature
