From cbb02c07924757794f611235ea16bee6717918c4 Mon Sep 17 00:00:00 2001 From: Tom Sepez Date: Wed, 29 Mar 2017 13:32:14 -0700 Subject: Avoid guessing vsnprintf() buffer length. This pattern is widely used in chrome. Change-Id: I64229b2ce298bf78773fe47c68d19be59411a76a Reviewed-on: https://pdfium-review.googlesource.com/3293 Commit-Queue: Tom Sepez Reviewed-by: dsinclair --- core/fxcrt/fx_basic_bstring.cpp | 205 ++----------------------------- core/fxcrt/fx_basic_bstring_unittest.cpp | 12 -- 2 files changed, 9 insertions(+), 208 deletions(-) diff --git a/core/fxcrt/fx_basic_bstring.cpp b/core/fxcrt/fx_basic_bstring.cpp index a4e491f4d3..f0052b6968 100644 --- a/core/fxcrt/fx_basic_bstring.cpp +++ b/core/fxcrt/fx_basic_bstring.cpp @@ -449,203 +449,16 @@ void CFX_ByteString::FormatV(const char* pFormat, va_list argList) { #else argListSave = argList; #endif - int nMaxLen = 0; - for (const char* pStr = pFormat; *pStr != 0; pStr++) { - if (*pStr != '%' || *(pStr = pStr + 1) == '%') { - nMaxLen += FXSYS_strlen(pStr); - continue; + FX_STRSIZE nMaxLen = vsnprintf(nullptr, 0, pFormat, argList); + if (nMaxLen > 0) { + GetBuffer(nMaxLen); + if (m_pData) { + // In the following two calls, there's always space in the buffer for + // a terminating NUL that's not included in nMaxLen. + memset(m_pData->m_String, 0, nMaxLen + 1); + vsnprintf(m_pData->m_String, nMaxLen + 1, pFormat, argListSave); + ReleaseBuffer(); } - int nItemLen = 0; - int nWidth = 0; - for (; *pStr != 0; pStr++) { - if (*pStr == '#') { - nMaxLen += 2; - } else if (*pStr == '*') { - nWidth = va_arg(argList, int); - } else if (*pStr != '-' && *pStr != '+' && *pStr != '0' && *pStr != ' ') { - break; - } - } - if (nWidth == 0) { - nWidth = FXSYS_atoi(pStr); - while (std::isdigit(*pStr)) - pStr++; - } - if (nWidth < 0 || nWidth > 128 * 1024) { - pFormat = "Bad width"; - nMaxLen = 10; - break; - } - int nPrecision = 0; - if (*pStr == '.') { - pStr++; - if (*pStr == '*') { - nPrecision = va_arg(argList, int); - pStr++; - } else { - nPrecision = FXSYS_atoi(pStr); - while (std::isdigit(*pStr)) - pStr++; - } - } - if (nPrecision < 0 || nPrecision > 128 * 1024) { - pFormat = "Bad precision"; - nMaxLen = 14; - break; - } - int nModifier = 0; - if (FXSYS_strncmp(pStr, "I64", 3) == 0) { - pStr += 3; - nModifier = FORCE_INT64; - } else { - switch (*pStr) { - case 'h': - nModifier = FORCE_ANSI; - pStr++; - break; - case 'l': - nModifier = FORCE_UNICODE; - pStr++; - break; - case 'F': - case 'N': - case 'L': - pStr++; - break; - } - } - switch (*pStr | nModifier) { - case 'c': - case 'C': - nItemLen = 2; - va_arg(argList, int); - break; - case 'c' | FORCE_ANSI: - case 'C' | FORCE_ANSI: - nItemLen = 2; - va_arg(argList, int); - break; - case 'c' | FORCE_UNICODE: - case 'C' | FORCE_UNICODE: - nItemLen = 2; - va_arg(argList, int); - break; - case 's': { - const char* pstrNextArg = va_arg(argList, const char*); - if (pstrNextArg) { - nItemLen = FXSYS_strlen(pstrNextArg); - if (nItemLen < 1) { - nItemLen = 1; - } - } else { - nItemLen = 6; - } - } break; - case 'S': { - wchar_t* pstrNextArg = va_arg(argList, wchar_t*); - if (pstrNextArg) { - nItemLen = FXSYS_wcslen(pstrNextArg); - if (nItemLen < 1) { - nItemLen = 1; - } - } else { - nItemLen = 6; - } - } break; - case 's' | FORCE_ANSI: - case 'S' | FORCE_ANSI: { - const char* pstrNextArg = va_arg(argList, const char*); - if (pstrNextArg) { - nItemLen = FXSYS_strlen(pstrNextArg); - if (nItemLen < 1) { - nItemLen = 1; - } - } else { - nItemLen = 6; - } - } break; - case 's' | FORCE_UNICODE: - case 'S' | FORCE_UNICODE: { - wchar_t* pstrNextArg = va_arg(argList, wchar_t*); - if (pstrNextArg) { - nItemLen = FXSYS_wcslen(pstrNextArg); - if (nItemLen < 1) { - nItemLen = 1; - } - } else { - nItemLen = 6; - } - } break; - } - if (nItemLen != 0) { - if (nPrecision != 0 && nItemLen > nPrecision) { - nItemLen = nPrecision; - } - if (nItemLen < nWidth) { - nItemLen = nWidth; - } - } else { - switch (*pStr) { - case 'd': - case 'i': - case 'u': - case 'x': - case 'X': - case 'o': - if (nModifier & FORCE_INT64) { - va_arg(argList, int64_t); - } else { - va_arg(argList, int); - } - nItemLen = 32; - if (nItemLen < nWidth + nPrecision) { - nItemLen = nWidth + nPrecision; - } - break; - case 'a': - case 'A': - case 'e': - case 'E': - case 'g': - case 'G': - va_arg(argList, double); - nItemLen = 128; - if (nItemLen < nWidth + nPrecision) { - nItemLen = nWidth + nPrecision; - } - break; - case 'f': - if (nWidth + nPrecision > 100) { - nItemLen = nPrecision + nWidth + 128; - } else { - char pszTemp[256]; - double f = va_arg(argList, double); - memset(pszTemp, 0, sizeof(pszTemp)); - FXSYS_snprintf(pszTemp, sizeof(pszTemp) - 1, "%*.*f", nWidth, - nPrecision + 6, f); - nItemLen = FXSYS_strlen(pszTemp); - } - break; - case 'p': - va_arg(argList, void*); - nItemLen = 32; - if (nItemLen < nWidth + nPrecision) { - nItemLen = nWidth + nPrecision; - } - break; - case 'n': - va_arg(argList, int*); - break; - } - } - nMaxLen += nItemLen; - } - nMaxLen += 32; // Fudge factor. - GetBuffer(nMaxLen); - if (m_pData) { - memset(m_pData->m_String, 0, nMaxLen); - FXSYS_vsnprintf(m_pData->m_String, nMaxLen - 1, pFormat, argListSave); - ReleaseBuffer(); } va_end(argListSave); } diff --git a/core/fxcrt/fx_basic_bstring_unittest.cpp b/core/fxcrt/fx_basic_bstring_unittest.cpp index a58169d039..e234989bf0 100644 --- a/core/fxcrt/fx_basic_bstring_unittest.cpp +++ b/core/fxcrt/fx_basic_bstring_unittest.cpp @@ -1047,12 +1047,6 @@ TEST(fxcrt, ByteStringFormatWidth) { str.Format("%0d", 1); EXPECT_EQ("1", str); } - - { - CFX_ByteString str; - str.Format("%1048576d", 1); - EXPECT_EQ("Bad width", str); - } } TEST(fxcrt, ByteStringFormatPrecision) { @@ -1085,12 +1079,6 @@ TEST(fxcrt, ByteStringFormatPrecision) { str.Format("%0f", 1.12345); EXPECT_EQ("1.123450", str); } - - { - CFX_ByteString str; - str.Format("%.1048576f", 1.2); - EXPECT_EQ("Bad precision", str); - } } TEST(fxcrt, EmptyByteString) { -- cgit v1.2.3