Summary

I spotted an unsafe use of the return value of vsnprintf() that leads to an out-of-bounds memory access in RIOT source code at:
https://github.com/RIOT-OS/RIOT/blob/master/cpu/esp_common/lib_printf.c#L47-L65

Details

The vsnprintf() API function returns the number of characters (excluding the terminating NUL byte) which would have been written to the destination string if enough space had been available. If an attacker is able to craft input so that the final string would become larger than PRINTF_BUFSIZ, they can exploit this bug to overwrite a '\n', '\r' or ' ' byte (or a sequence of such bytes) with a NUL byte in memory past the end of the _printf_buf static buffer. In addition, the tainted len value is returned at the end of the _lib_printf() function and may be used unsafely (e.g., as an array index) somewhere else in the code.

Please refer to the marked lines below:

static char _printf_buf[PRINTF_BUFSIZ];

static int _lib_printf(int level, const char* tag, const char* format, va_list arg)
{
    if (level > LOG_LEVEL) {
        return 0;
    }

    int len = vsnprintf(_printf_buf, PRINTF_BUFSIZ - 1, format, arg); // VULN: vsnprintf() returns the total length of the string it tried to create, which may be larger than the actual size of the destination string

    /*
     * Since ESP_EARLY_LOG macros add a line break at the end, a terminating
     * line break in the output must be removed if there is one.
     */
    _printf_buf[PRINTF_BUFSIZ - 1] = 0;
    int i;
    for (i = len - 1; i >= 0; --i) {
        if (_printf_buf[i] != '\n' && _printf_buf[i] != '\r' && _printf_buf[i] != ' ') {
            break;
        }
        _printf_buf[i] = 0; // VULN: very limited oob array write access
    }
    if (i > 0) {
        ESP_EARLY_LOGI(tag, "%s", _printf_buf);
    }
    va_end(arg);
    return len; // VULN: tainted len value is returned
}

Impact

In my understanding, the impact of this vulnerability in this specific case is quite limited. However, all bugs that have the potential to cause memory corruption should be taken seriously and fixed as security bugs.