Project

General

Profile

Actions

Bug #408

closed

Bug #438: Release v1.4

flashbuses_to_text() may crash flashrom

Added by Alexander Goncharov over 1 year ago. Updated 12 months ago.

Status:
Resolved
Priority:
Normal
Category:
-
Target version:
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;
}
Actions #1

Updated by Felix Singer over 1 year ago

  • Parent task set to #353
Actions #2

Updated by Felix Singer over 1 year ago

How did you encounter the bug? How can one reproduce it? Please give some more details.

Actions #3

Updated by Edward . over 1 year ago

Actions #4

Updated by Alexander Goncharov over 1 year 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.

Actions #5

Updated by Felix Singer over 1 year ago

  • Assignee changed from Alexander Goncharov to Felix Singer
Actions #6

Updated by Anastasia Klimchuk over 1 year ago

  • Assignee changed from Felix Singer to Alexander Goncharov
  • Parent task changed from #353 to #438
Actions #8

Updated by Anastasia Klimchuk 12 months ago

  • Status changed from New to Resolved

Fixed (see patch above)

Actions

Also available in: Atom PDF