Actions
Bug #408
closedBug #438: Release v1.4
flashbuses_to_text() may crash flashrom
Start date:
08/22/2022
Due date:
% Done:
0%
Estimated time:
Affected versions:
Needs backport to:
Affected hardware:
Affected OS:
Description
See the provided comments in the function code towards the end.
/*
* Return a string corresponding to the bustype parameter.
* Memory is obtained with malloc() and must be freed with free() by the caller.
*/
char *flashbuses_to_text(enum chipbustype bustype)
{
char *ret = calloc(1, 1);
/*
* FIXME: Once all chipsets and flash chips have been updated, NONSPI
* will cease to exist and should be eliminated here as well.
*/
if (bustype == BUS_NONSPI) {
ret = strcat_realloc(ret, "Non-SPI, ");
} else {
if (bustype & BUS_PARALLEL)
ret = strcat_realloc(ret, "Parallel, ");
if (bustype & BUS_LPC)
ret = strcat_realloc(ret, "LPC, ");
if (bustype & BUS_FWH)
ret = strcat_realloc(ret, "FWH, ");
if (bustype & BUS_SPI)
ret = strcat_realloc(ret, "SPI, ");
if (bustype & BUS_PROG)
ret = strcat_realloc(ret, "Programmer-specific, ");
if (bustype == BUS_NONE)
ret = strcat_realloc(ret, "None, ");
}
// BUG:
// If ret == NULL right now, then flashrom will crash.
// strlen(ret) will cause a segmentation fault.
/* Kill last comma. */
ret[strlen(ret) - 2] = '\0';
ret = realloc(ret, strlen(ret) + 1);
return ret;
}
Updated by Felix Singer about 2 years ago
How did you encounter the bug? How can one reproduce it? Please give some more details.
Updated by Edward . about 2 years ago
Is this what you want? https://review.coreboot.org/c/flashrom/+/69219
Updated by Alexander Goncharov about 2 years ago
I found this bug when I just went through the code. strcat_realloc()
can return NULL, but we don't handle this case. So, the bug will appear only when the PC has no available memory (out of memory).
Edward, your implementation is interesting but still has some memory problems. The first calloc()
, which initializes char *ret
, can fail and returns NULL
; strlen(NULL)
will cause an error.
Updated by Felix Singer about 2 years ago
- Assignee changed from Alexander Goncharov to Felix Singer
Updated by Anastasia Klimchuk about 2 years ago
- Assignee changed from Felix Singer to Alexander Goncharov
- Parent task changed from #353 to #438
Updated by Alexander Goncharov almost 2 years ago
Updated by Anastasia Klimchuk over 1 year ago
- Status changed from New to Resolved
Fixed (see patch above)
Actions