Hi all,
On Tue, Oct 11, 2022 at 7:03 PM Angel Pons th3fanbus@gmail.com wrote:
Hi Patrick,
On Tue, Oct 11, 2022 at 6:43 PM Patrick Georgi patrick@coreboot.org wrote:
"Angel Pons" th3fanbus@gmail.com schrieb:
We made the patches that made Coverity angry about this `format_pn()` function. However, this is not an actual bug: the `eeprom_read_serial()` function returns a buffer that is at most 32 (`HERMES_SN_PN_LENGTH`) characters long, and the length of the `prefix` string is known at build-time (it's a string literal in both call sites) to be less than 32 characters long.
There's no guarantee that the string returned by eeprom_read_serial() is 0-terminated (not even in its implementation) and strcpy proceeds until the first 0 it sees, even if that's only 2GB later. Use strncpy instead to prevent out of bound copies.
Oh, we wanted to use `strncat()` but it doesn't exist in coreboot. Then we considered `strcat()` and it didn't exist either, and we ended up using `strcpy()`. Will use `strncpy()`, thank you!
After some discussion on IRC, we ended up making https://review.coreboot.org/68322 and https://review.coreboot.org/68323 to address the issues. Thank you all.
Patrick
Best regards, Angel
Best regards, Angel