rockchip-linux / rkdeveloptool

GNU General Public License v2.0
146 stars 85 forks source link

Fix Potential Buffer Overflow in convertChipType Function #93

Open VasenevEA opened 11 months ago

VasenevEA commented 11 months ago

Issue:

The original implementation of the convertChipType function in main.cpp was flagged by the compiler warning, suggesting the possibility of a buffer overflow due to the use of snprintf writing more characters than the buffer size.

main.cpp:1541:36: error: ‘%s’ directive output may be truncated writing up to 557 bytes into a region of size 5 [-Werror=format-truncation=]
 1541 |  snprintf(buffer, sizeof(buffer), "%s", chip);
      |                                    ^~

Fix:

To address this, the function has been updated to first check the length of the chip string. If the length is greater than 4 (the intended size of the buffer minus the null terminator), the function returns 0. This ensures that snprintf never attempts to write more characters than the buffer can handle.

Changes Made:

Added a check to ensure chip string length does not exceed 4 characters. Returned 0 if the string length check fails. The updated code is as follows:

static inline uint32_t convertChipType(const char* chip) {
    if (strlen(chip) > 4) {
        return 0;
    }

    char buffer[5];
    memset(buffer, 0, sizeof(buffer));
    snprintf(buffer, sizeof(buffer), "%s", chip);
    return buffer[0] << 24 | buffer[1] << 16 | buffer[2] << 8 | buffer[3];
}

This change should prevent any potential buffer overflows

https://github.com/rockchip-linux/rkdeveloptool/issues/47

RadxaYuntian commented 10 months ago

85 might be the intended behavior. The original function never returns 0.