ImFormatStringToTempBufferV crashes if a null is passed for a %s parameter #7016

New issue

Closed

opened 2023-11-15 21:46:09 +00:00 by codefrog2002 · 0 comments

codefrog2002 commented

2023-11-15 21:46:09 +00:00

(Migrated from github.com)

Version: 1.90
Branch: master

Many printf implementations will accept a nullptr when passed to a %s format.
That is, printf("%s", nullptr); will emit "(null)" on many implementations of printf.
I recognize this is undefined behavior, but it's easy to handle it gracefully.

ImFormatStringToTempBufferV instead crashes during its attempt to optimize around the heavy ImFormatStringV.

See this block of code - ImGui.cpp, void ImFormatStringToTempBufferV

    if (fmt[0] == '%' && fmt[1] == 's' && fmt[2] == 0)
    {
        const char* buf = va_arg(args, const char*); // Skip formatting when using "%s"
        
        if (buf == nullptr) buf = "(null)";  // << I added this to prevent crash from calling strlen(nullptr)
        
        *out_buf = buf;
        if (out_buf_end) { *out_buf_end = buf + strlen(buf); }
    }

And as of the newest version (1.90) there's a subsequent code block (looking for "%.*s") with the same issue.

    else if (fmt[0] == '%' && fmt[1] == '.' && fmt[2] == '*' && fmt[3] == 's' && fmt[4] == 0)
    {
        int buf_len = va_arg(args, int); // Skip formatting when using "%.*s"
        const char* buf = va_arg(args, const char*);
        if (buf == nullptr) buf = "(null)"; // Added  to prevent nullptr + length 
        *out_buf = buf;
        *out_buf_end = buf + buf_len; // Disallow not passing 'out_buf_end' here. User is expected to use it.
    }

Version: 1.90 Branch: master Many printf implementations will accept a nullptr when passed to a %s format. That is, `printf("%s", nullptr);` will emit `"(null)"` on many implementations of printf. I recognize this is undefined behavior, but it's easy to handle it gracefully. ImFormatStringToTempBufferV instead crashes during its attempt to optimize around the heavy ImFormatStringV. See this block of code - `ImGui.cpp, void ImFormatStringToTempBufferV` ``` if (fmt[0] == '%' && fmt[1] == 's' && fmt[2] == 0) { const char* buf = va_arg(args, const char*); // Skip formatting when using "%s" if (buf == nullptr) buf = "(null)"; // << I added this to prevent crash from calling strlen(nullptr) *out_buf = buf; if (out_buf_end) { *out_buf_end = buf + strlen(buf); } } ``` And as of the newest version (1.90) there's a subsequent code block (looking for "%.*s") with the same issue. ``` else if (fmt[0] == '%' && fmt[1] == '.' && fmt[2] == '*' && fmt[3] == 's' && fmt[4] == 0) { int buf_len = va_arg(args, int); // Skip formatting when using "%.*s" const char* buf = va_arg(args, const char*); if (buf == nullptr) buf = "(null)"; // Added to prevent nullptr + length *out_buf = buf; *out_buf_end = buf + buf_len; // Disallow not passing 'out_buf_end' here. User is expected to use it. } ```