rkdeveloptool icon indicating copy to clipboard operation
rkdeveloptool copied to clipboard

Fix Potential Buffer Overflow in convertChipType Function

Open VasenevEA opened this issue 1 year ago • 1 comments

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

VasenevEA avatar Aug 23 '23 20:08 VasenevEA

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

RadxaYuntian avatar Aug 28 '23 01:08 RadxaYuntian