Mark unsafe_buffer regions in fgas/{crt,font}
-- Convert to std::array<> in one spot.
-- Punt on multidimensional std::arrays
-- Mark wcscmp() as UNSAFE_TODO(), hope that it will be re-written
someday to use more modern string operations.
Bug: 42271175
Change-Id: Id610756863936bff7034f196c0f7ade0f8892558
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/119590
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Thomas Sepez <tsepez@google.com>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/xfa/fgas/crt/cfgas_stringformatter.cpp b/xfa/fgas/crt/cfgas_stringformatter.cpp
index 325cfc1..62e814c 100644
--- a/xfa/fgas/crt/cfgas_stringformatter.cpp
+++ b/xfa/fgas/crt/cfgas_stringformatter.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "xfa/fgas/crt/cfgas_stringformatter.h"
#include <math.h>
@@ -19,10 +14,12 @@
#include <vector>
#include "core/fxcrt/cfx_datetime.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/containers/contains.h"
#include "core/fxcrt/fx_extension.h"
#include "core/fxcrt/fx_safe_types.h"
#include "core/fxcrt/notreached.h"
+#include "core/fxcrt/stl_util.h"
#include "xfa/fgas/crt/cfgas_decimal.h"
#include "xfa/fgas/crt/locale_mgr_iface.h"
@@ -70,10 +67,17 @@
int16_t iMinute;
};
-constexpr FX_LOCALETIMEZONEINFO kFXLocaleTimeZoneData[] = {
- {L"CDT", -5, 0}, {L"CST", -6, 0}, {L"EDT", -4, 0}, {L"EST", -5, 0},
- {L"MDT", -6, 0}, {L"MST", -7, 0}, {L"PDT", -7, 0}, {L"PST", -8, 0},
-};
+constexpr auto kFXLocaleTimeZoneData =
+ fxcrt::ToArray<const FX_LOCALETIMEZONEINFO>({
+ {L"CDT", -5, 0},
+ {L"CST", -6, 0},
+ {L"EDT", -4, 0},
+ {L"EST", -5, 0},
+ {L"MDT", -6, 0},
+ {L"MST", -7, 0},
+ {L"PDT", -7, 0},
+ {L"PST", -8, 0},
+ });
constexpr wchar_t kTimeSymbols[] = L"hHkKMSFAzZ";
constexpr wchar_t kDateSymbols[] = L"DJMEeGgYwW";
@@ -257,7 +261,8 @@
WideString wsLiteral = GetLiteralText(spDatePattern, &ccf);
size_t iLiteralLen = wsLiteral.GetLength();
if (*cc + iLiteralLen > spDate.size() ||
- wcsncmp(spDate.data() + *cc, wsLiteral.c_str(), iLiteralLen) != 0) {
+ UNSAFE_TODO(wcsncmp(spDate.data() + *cc, wsLiteral.c_str(),
+ iLiteralLen)) != 0) {
return false;
}
*cc += iLiteralLen;
@@ -295,8 +300,8 @@
pLocale->GetMonthName(i, symbol.EqualsASCII("MMM"));
if (wsMonthName.IsEmpty())
continue;
- if (wcsncmp(wsMonthName.c_str(), spDate.data() + *cc,
- wsMonthName.GetLength()) == 0) {
+ if (UNSAFE_TODO(wcsncmp(wsMonthName.c_str(), spDate.data() + *cc,
+ wsMonthName.GetLength())) == 0) {
*cc += wsMonthName.GetLength();
month = i + 1;
break;
@@ -308,8 +313,8 @@
pLocale->GetDayName(i, symbol.EqualsASCII("EEE"));
if (wsDayName.IsEmpty())
continue;
- if (wcsncmp(wsDayName.c_str(), spDate.data() + *cc,
- wsDayName.GetLength()) == 0) {
+ if (UNSAFE_TODO(wcsncmp(wsDayName.c_str(), spDate.data() + *cc,
+ wsDayName.GetLength())) == 0) {
*cc += wsDayName.GetLength();
break;
}
@@ -376,7 +381,8 @@
WideString wsLiteral = GetLiteralText(spTimePattern, &ccf);
size_t iLiteralLen = wsLiteral.GetLength();
if (*cc + iLiteralLen > spTime.size() ||
- wcsncmp(spTime.data() + *cc, wsLiteral.c_str(), iLiteralLen) != 0) {
+ UNSAFE_TODO(wcsncmp(spTime.data() + *cc, wsLiteral.c_str(),
+ iLiteralLen)) != 0) {
return false;
}
*cc += iLiteralLen;
@@ -434,12 +440,13 @@
WideString wsAM = pLocale->GetMeridiemName(true);
WideString wsPM = pLocale->GetMeridiemName(false);
if (*cc + wsAM.GetLength() <= spTime.size() &&
- WideStringView(spTime.data() + *cc, wsAM.GetLength()) == wsAM) {
+ UNSAFE_TODO(WideStringView(spTime.data() + *cc, wsAM.GetLength())) ==
+ wsAM) {
*cc += wsAM.GetLength();
bHasA = true;
} else if (*cc + wsPM.GetLength() <= spTime.size() &&
- WideStringView(spTime.data() + *cc, wsPM.GetLength()) ==
- wsPM) {
+ UNSAFE_TODO(WideStringView(spTime.data() + *cc,
+ wsPM.GetLength())) == wsPM) {
*cc += wsPM.GetLength();
bHasA = true;
bPM = true;
@@ -528,7 +535,8 @@
}
uint16_t GetWeekDay(uint16_t year, uint16_t month, uint16_t day) {
- static const uint8_t kMonthDay[] = {0, 3, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5};
+ static constexpr auto kMonthDay =
+ fxcrt::ToArray<const uint8_t>({0, 3, 3, 6, 1, 4, 6, 2, 5, 0, 3, 5});
uint16_t nDays =
(year - 1) % 7 + (year - 1) / 4 - (year - 1) / 100 + (year - 1) / 400;
nDays += kMonthDay[month - 1] + day;
@@ -876,11 +884,13 @@
if (spFormatString[index] == '\'') {
bQuote = !bQuote;
} else if (spFormatString[index] == L'|' && !bQuote) {
- wsPatterns.emplace_back(spFormatString.data() + token, index - token);
+ UNSAFE_TODO(wsPatterns.emplace_back(spFormatString.data() + token,
+ index - token));
token = index + 1;
}
}
- wsPatterns.emplace_back(spFormatString.data() + token, index - token);
+ UNSAFE_TODO(
+ wsPatterns.emplace_back(spFormatString.data() + token, index - token));
return wsPatterns;
}
@@ -943,8 +953,8 @@
if (m_spPattern[ccf] == '\'') {
size_t iCurChar = ccf;
GetLiteralText(m_spPattern, &ccf);
- wsPurgePattern +=
- WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1);
+ wsPurgePattern += UNSAFE_TODO(
+ WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1));
} else if (!bBrackOpen &&
!pdfium::Contains(kConstChars, m_spPattern[ccf])) {
WideString wsSearchCategory(m_spPattern[ccf]);
@@ -994,8 +1004,8 @@
if (m_spPattern[ccf] == '\'') {
size_t iCurChar = ccf;
GetLiteralText(m_spPattern, &ccf);
- *wsPurgePattern +=
- WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1);
+ *wsPurgePattern += UNSAFE_TODO(
+ WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1));
} else if (!bBrackOpen &&
!pdfium::Contains(kConstChars, m_spPattern[ccf])) {
WideString wsCategory(m_spPattern[ccf]);
@@ -1102,8 +1112,8 @@
WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
- wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen) !=
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(),
+ iLiteralLen)) != 0) {
*wsValue = wsSrcText;
return false;
}
@@ -1205,8 +1215,8 @@
size_t iLiteralLen = wsLiteral.GetLength();
cc -= iLiteralLen - 1;
if (cc >= spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsLiteral.c_str(), iLiteralLen) !=
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsLiteral.c_str(),
+ iLiteralLen)) != 0) {
return false;
}
cc--;
@@ -1241,7 +1251,8 @@
} else {
cc -= iMinusLen - 1;
if (cc >= spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsMinus.c_str(), iMinusLen) != 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsMinus.c_str(),
+ iMinusLen)) != 0) {
return false;
}
cc--;
@@ -1267,8 +1278,8 @@
continue;
}
if (cc - iMinusLen + 1 <= spSrcNum.size() &&
- wcsncmp(spSrcNum.data() + (cc - iMinusLen + 1), wsMinus.c_str(),
- iMinusLen) == 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + (cc - iMinusLen + 1),
+ wsMinus.c_str(), iMinusLen)) == 0) {
bExpSign = true;
cc -= iMinusLen;
continue;
@@ -1286,7 +1297,8 @@
size_t iSymbolLen = wsSymbol.GetLength();
cc -= iSymbolLen - 1;
if (cc >= spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(), iSymbolLen) != 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(),
+ iSymbolLen)) != 0) {
return false;
}
cc--;
@@ -1332,7 +1344,8 @@
size_t iSymbolLen = wsSymbol.GetLength();
cc -= iSymbolLen - 1;
if (cc >= spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(), iSymbolLen) != 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(),
+ iSymbolLen)) != 0) {
return false;
}
cc--;
@@ -1349,8 +1362,8 @@
if (cc < spSrcNum.size()) {
cc -= iGroupLen - 1;
if (cc < spSrcNum.size() &&
- wcsncmp(spSrcNum.data() + cc, wsGroupSymbol.c_str(), iGroupLen) ==
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsGroupSymbol.c_str(),
+ iGroupLen)) == 0) {
cc--;
} else {
cc += iGroupLen - 1;
@@ -1397,8 +1410,8 @@
WideString wsLiteral = GetLiteralText(spNumFormat, &ccf);
size_t iLiteralLen = wsLiteral.GetLength();
if (cc + iLiteralLen > spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsLiteral.c_str(), iLiteralLen) !=
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsLiteral.c_str(),
+ iLiteralLen)) != 0) {
return false;
}
cc += iLiteralLen;
@@ -1429,8 +1442,8 @@
cc++;
} else {
if (cc + iMinusLen > spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsMinus.c_str(), iMinusLen) !=
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsMinus.c_str(),
+ iMinusLen)) != 0) {
return false;
}
bNeg = true;
@@ -1469,8 +1482,8 @@
WideString wsSymbol = pLocale->GetCurrencySymbol();
size_t iSymbolLen = wsSymbol.GetLength();
if (cc + iSymbolLen > spSrcNum.size() ||
- wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(), iSymbolLen) !=
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(),
+ iSymbolLen)) != 0) {
return false;
}
cc += iSymbolLen;
@@ -1514,8 +1527,8 @@
WideString wsSymbol = pLocale->GetPercentSymbol();
size_t iSymbolLen = wsSymbol.GetLength();
if (cc + iSymbolLen <= spSrcNum.size() &&
- wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(), iSymbolLen) ==
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsSymbol.c_str(),
+ iSymbolLen)) == 0) {
cc += iSymbolLen;
}
bHavePercentSymbol = true;
@@ -1531,8 +1544,8 @@
} break;
case ',': {
if (cc + iGroupLen <= spSrcNum.size() &&
- wcsncmp(spSrcNum.data() + cc, wsGroupSymbol.c_str(), iGroupLen) ==
- 0) {
+ UNSAFE_TODO(wcsncmp(spSrcNum.data() + cc, wsGroupSymbol.c_str(),
+ iGroupLen)) == 0) {
cc += iGroupLen;
}
break;
@@ -1585,8 +1598,8 @@
if (m_spPattern[ccf] == '\'') {
size_t iCurChar = ccf;
GetLiteralText(m_spPattern, &ccf);
- wsTempPattern +=
- WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1);
+ wsTempPattern += UNSAFE_TODO(
+ WideStringView(m_spPattern.data() + iCurChar, ccf - iCurChar + 1));
} else if (!bBraceOpen && eDateTimeType != DateTimeType::kDateTime &&
!pdfium::Contains(kConstChars, m_spPattern[ccf])) {
WideString wsCategory(m_spPattern[ccf]);
@@ -1754,7 +1767,8 @@
WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
- wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen)) {
+ UNSAFE_TODO(wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(),
+ iLiteralLen))) {
return false;
}
iText += iLiteralLen;
@@ -1782,7 +1796,8 @@
WideString wsLiteral = GetLiteralText(spTextFormat, &iPattern);
size_t iLiteralLen = wsLiteral.GetLength();
if (iText + iLiteralLen > spSrcText.size() ||
- wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(), iLiteralLen)) {
+ UNSAFE_TODO(wcsncmp(spSrcText.data() + iText, wsLiteral.c_str(),
+ iLiteralLen))) {
return false;
}
iText += iLiteralLen;
diff --git a/xfa/fgas/font/cfgas_defaultfontmanager.cpp b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
index 1410ee1..7ff89d6 100644
--- a/xfa/fgas/font/cfgas_defaultfontmanager.cpp
+++ b/xfa/fgas/font/cfgas_defaultfontmanager.cpp
@@ -4,13 +4,9 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "xfa/fgas/font/cfgas_defaultfontmanager.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_codepage.h"
#include "core/fxcrt/numerics/safe_conversions.h"
#include "core/fxge/fx_font.h"
@@ -46,7 +42,7 @@
while (iLength > 0) {
const char* pNameText = pReplace;
while (*pNameText != ',' && iLength > 0) {
- pNameText++;
+ UNSAFE_TODO(pNameText++);
iLength--;
}
WideString wsReplace =
@@ -57,7 +53,7 @@
break;
iLength--;
- pNameText++;
+ UNSAFE_TODO(pNameText++);
pReplace = pNameText;
}
return pFont;
diff --git a/xfa/fgas/font/cfgas_fontmgr.cpp b/xfa/fgas/font/cfgas_fontmgr.cpp
index 68b08cb..47fdec9 100644
--- a/xfa/fgas/font/cfgas_fontmgr.cpp
+++ b/xfa/fgas/font/cfgas_fontmgr.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "xfa/fgas/font/cfgas_fontmgr.h"
#include <stdint.h>
@@ -193,9 +188,11 @@
font.uCharSet = FX_GetCharsetFromInt(lf.lfCharSet);
font.dwFontStyles = GetGdiFontStyles(lf);
FXSYS_wcsncpy(font.wsFontFace, (const wchar_t*)lf.lfFaceName, 31);
- font.wsFontFace[31] = 0;
- FXSYS_memcpy(&font.FontSignature, &lpntme->ntmFontSig,
- sizeof(lpntme->ntmFontSig));
+ UNSAFE_TODO({
+ font.wsFontFace[31] = 0;
+ FXSYS_memcpy(&font.FontSignature, &lpntme->ntmFontSig,
+ sizeof(lpntme->ntmFontSig));
+ });
reinterpret_cast<std::deque<FX_FONTDESCRIPTOR>*>(lParam)->push_back(font);
return 1;
}
@@ -213,8 +210,10 @@
static_assert(std::is_aggregate_v<decltype(lfFind)>);
lfFind.lfCharSet = DEFAULT_CHARSET;
if (pwsFaceName) {
- FXSYS_wcsncpy(lfFind.lfFaceName, pwsFaceName, 31);
- lfFind.lfFaceName[31] = 0;
+ UNSAFE_TODO({
+ FXSYS_wcsncpy(lfFind.lfFaceName, pwsFaceName, 31);
+ lfFind.lfFaceName[31] = 0;
+ });
}
HDC hDC = ::GetDC(nullptr);
EnumFontFamiliesExW(hDC, (LPLOGFONTW)&lfFind, (FONTENUMPROCW)GdiFontEnumProc,
@@ -376,8 +375,9 @@
uint16_t FX_GetCodePageBit(FX_CodePage wCodePage) {
for (size_t i = 0; i < std::size(kCodePages); ++i) {
- if (kCodePages[i] == wCodePage)
+ if (UNSAFE_TODO(kCodePages[i]) == wCodePage) {
return static_cast<uint16_t>(i);
+ }
}
return static_cast<uint16_t>(-1);
}
@@ -520,7 +520,7 @@
// https://bugs.chromium.org/p/pdfium/issues/detail?id=690
FXFT_StreamRec* ftStream =
static_cast<FXFT_StreamRec*>(ft_scalloc(sizeof(FXFT_StreamRec), 1));
- FXSYS_memset(ftStream, 0, sizeof(FXFT_StreamRec));
+ UNSAFE_TODO(FXSYS_memset(ftStream, 0, sizeof(FXFT_StreamRec)));
ftStream->base = nullptr;
ftStream->descriptor.pointer = static_cast<void*>(pFontStream.Get());
ftStream->pos = 0;
diff --git a/xfa/fgas/font/cfgas_fontmgr.h b/xfa/fgas/font/cfgas_fontmgr.h
index f865501..e91a67a 100644
--- a/xfa/fgas/font/cfgas_fontmgr.h
+++ b/xfa/fgas/font/cfgas_fontmgr.h
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#ifndef XFA_FGAS_FONT_CFGAS_FONTMGR_H_
#define XFA_FGAS_FONT_CFGAS_FONTMGR_H_
@@ -20,6 +15,7 @@
#include <vector>
#include "build/build_config.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxcrt/fx_codepage_forward.h"
#include "core/fxcrt/retain_ptr.h"
#include "core/fxcrt/widestring.h"
@@ -30,8 +26,8 @@
#if BUILDFLAG(IS_WIN)
struct FX_FONTSIGNATURE {
- uint32_t fsUsb[4];
- uint32_t fsCsb[2];
+ std::array<uint32_t, 4> fsUsb;
+ std::array<uint32_t, 2> fsCsb;
};
inline bool operator==(const FX_FONTSIGNATURE& left,
diff --git a/xfa/fgas/font/cfgas_pdffontmgr.cpp b/xfa/fgas/font/cfgas_pdffontmgr.cpp
index 9c45b31..668591b 100644
--- a/xfa/fgas/font/cfgas_pdffontmgr.cpp
+++ b/xfa/fgas/font/cfgas_pdffontmgr.cpp
@@ -4,11 +4,6 @@
// Original code copyright 2014 Foxit Software Inc. http://www.foxitsoftware.com
-#if defined(UNSAFE_BUFFERS_BUILD)
-// TODO(crbug.com/pdfium/2154): resolve buffer safety issues.
-#pragma allow_unsafe_buffers
-#endif
-
#include "xfa/fgas/font/cfgas_pdffontmgr.h"
#include <algorithm>
@@ -21,6 +16,7 @@
#include "core/fpdfapi/parser/cpdf_document.h"
#include "core/fpdfapi/parser/fpdf_parser_utility.h"
#include "core/fxcrt/check.h"
+#include "core/fxcrt/compiler_specific.h"
#include "core/fxge/fx_font.h"
#include "xfa/fgas/font/cfgas_fontmgr.h"
#include "xfa/fgas/font/cfgas_gefont.h"
@@ -107,13 +103,13 @@
bool bBold,
bool bItalic) {
for (size_t i = 0; i < std::size(kXFAPDFFontName); ++i) {
- if (strPsName == kXFAPDFFontName[i][0]) {
+ if (strPsName == UNSAFE_TODO(kXFAPDFFontName[i][0])) {
size_t index = 1;
if (bBold)
++index;
if (bItalic)
index += 2;
- return kXFAPDFFontName[i][index];
+ return UNSAFE_TODO(kXFAPDFFontName[i][index]);
}
}
return strPsName;
