Mass convert memset() to FXSYS_memset(). The FXSYS_ form requires callers to specify UNSAFE_BUFFERS(). Most affected files are already in the pdfium_unsafe_buffers_paths.txt suppression file. Other usage is hand-patched with TODO()s to investigate safety. -- convert one fxcrt::Fill() missed in previous cl. Change-Id: I354a530da2439de8bbe7edc9a889857c6b8a4150 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/118390 Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org> Reviewed-by: Thomas Sepez <tsepez@google.com>
diff --git a/core/fdrm/fx_crypt_sha.cpp b/core/fdrm/fx_crypt_sha.cpp index 9c2bc96..cab3580 100644 --- a/core/fdrm/fx_crypt_sha.cpp +++ b/core/fdrm/fx_crypt_sha.cpp
@@ -407,7 +407,7 @@ } else { pad = 56 - context->blkused; } - memset(c, 0, pad); + FXSYS_memset(c, 0, pad); c[0] = 0x80; CRYPT_SHA1Update(context, c, pad); c[0] = (total_bits >> 56) & 0xFF;
diff --git a/core/fpdfapi/font/cpdf_cmap.cpp b/core/fpdfapi/font/cpdf_cmap.cpp index e2dd1d7..72c5196 100644 --- a/core/fpdfapi/font/cpdf_cmap.cpp +++ b/core/fpdfapi/font/cpdf_cmap.cpp
@@ -19,6 +19,7 @@ #include "core/fpdfapi/font/cpdf_fontglobals.h" #include "core/fpdfapi/parser/cpdf_simple_parser.h" #include "core/fxcrt/check.h" +#include "core/fxcrt/fx_memcpy_wrappers.h" namespace { @@ -482,7 +483,7 @@ iSize = 1; str[iSize - 1] = static_cast<char>(charcode); if (iSize > 1) - memset(str, 0, iSize - 1); + FXSYS_memset(str, 0, iSize - 1); return iSize; } if (charcode < 0x10000) {
diff --git a/core/fpdfapi/parser/cpdf_crypto_handler.cpp b/core/fpdfapi/parser/cpdf_crypto_handler.cpp index e3fd40f..7851f3a 100644 --- a/core/fpdfapi/parser/cpdf_crypto_handler.cpp +++ b/core/fpdfapi/parser/cpdf_crypto_handler.cpp
@@ -87,8 +87,8 @@ nblocks * 16); uint8_t padding[16]; FXSYS_memcpy(padding, source.data() + nblocks * 16, source.size() % 16); - memset(padding + source.size() % 16, 16 - source.size() % 16, - 16 - source.size() % 16); + FXSYS_memset(padding + source.size() % 16, 16 - source.size() % 16, + 16 - source.size() % 16); CRYPT_AESEncrypt(m_pAESContext.get(), dest_buf + nblocks * 16 + 16, padding, 16); dest_size = 32 + nblocks * 16;
diff --git a/core/fpdfapi/parser/cpdf_security_handler.cpp b/core/fpdfapi/parser/cpdf_security_handler.cpp index d0c8bf4..c932d63 100644 --- a/core/fpdfapi/parser/cpdf_security_handler.cpp +++ b/core/fpdfapi/parser/cpdf_security_handler.cpp
@@ -80,7 +80,7 @@ CRYPT_MD5Generate(pdfium::make_span(digest).first(copy_len), digest); } } - memset(key, 0, keylen); + FXSYS_memset(key, 0, keylen); FXSYS_memcpy(key, digest, copy_len); }
diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index 52400c1..3f04681 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp
@@ -27,8 +27,10 @@ #include "core/fxcrt/cfx_read_only_vector_stream.h" #include "core/fxcrt/check.h" #include "core/fxcrt/check_op.h" +#include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/fixed_size_data_vector.h" #include "core/fxcrt/fx_extension.h" +#include "core/fxcrt/fx_memcpy_wrappers.h" #include "core/fxcrt/fx_safe_types.h" namespace { @@ -748,7 +750,9 @@ if (len >= 0) { CPDF_ReadValidator::ScopedSession read_session(GetValidator()); m_Pos += ReadEOLMarkers(GetPos()); - memset(m_WordBuffer.data(), 0, kEndStreamStr.GetLength() + 1); + // TODO(tsepez): investigate safety. + UNSAFE_BUFFERS( + FXSYS_memset(m_WordBuffer.data(), 0, kEndStreamStr.GetLength() + 1)); GetNextWordInternal(); if (GetValidator()->has_read_problems()) return nullptr; @@ -807,7 +811,9 @@ stream = pdfium::MakeRetain<CPDF_Stream>(std::move(pDict)); } const FX_FILESIZE end_stream_offset = GetPos(); - memset(m_WordBuffer.data(), 0, kEndObjStr.GetLength() + 1); + // TODO(tsepez): investigate safety. + UNSAFE_BUFFERS( + FXSYS_memset(m_WordBuffer.data(), 0, kEndObjStr.GetLength() + 1)); GetNextWordInternal(); // Allow whitespace after endstream and before a newline.
diff --git a/core/fpdfapi/parser/fpdf_parser_decode.cpp b/core/fpdfapi/parser/fpdf_parser_decode.cpp index d18ead6..87adf8b 100644 --- a/core/fpdfapi/parser/fpdf_parser_decode.cpp +++ b/core/fpdfapi/parser/fpdf_parser_decode.cpp
@@ -30,6 +30,7 @@ #include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/containers/contains.h" #include "core/fxcrt/fx_extension.h" +#include "core/fxcrt/fx_memcpy_wrappers.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/span.h" #include "core/fxcrt/span_util.h" @@ -161,7 +162,7 @@ continue; if (ch == 'z') { - memset(dest_buf_ptr + *dest_size, 0, 4); + FXSYS_memset(dest_buf_ptr + *dest_size, 0, 4); state = 0; res = 0; *dest_size += 4;
diff --git a/core/fxcodec/fax/faxmodule.cpp b/core/fxcodec/fax/faxmodule.cpp index c285b84..6b2e774 100644 --- a/core/fxcodec/fax/faxmodule.cpp +++ b/core/fxcodec/fax/faxmodule.cpp
@@ -31,6 +31,7 @@ #include "core/fxcrt/raw_span.h" #include "core/fxcrt/span.h" #include "core/fxcrt/span_util.h" +#include "core/fxcrt/stl_util.h" #include "core/fxge/calculate_pitch.h" #if BUILDFLAG(IS_WIN) @@ -151,7 +152,7 @@ dest_buf[last_byte] -= 1 << (7 - i); if (last_byte > first_byte + 1) - memset(dest_buf + first_byte + 1, 0, last_byte - first_byte - 1); + FXSYS_memset(dest_buf + first_byte + 1, 0, last_byte - first_byte - 1); } inline bool NextBit(const uint8_t* src_buf, int* bitpos) { @@ -535,7 +536,7 @@ } bool FaxDecoder::Rewind() { - memset(m_RefBuf.data(), 0xff, m_RefBuf.size()); + fxcrt::Fill(m_RefBuf, 0xff); m_bitpos = 0; return true; } @@ -546,7 +547,7 @@ if (m_bitpos >= bitsize) return pdfium::span<uint8_t>(); - memset(m_ScanlineBuf.data(), 0xff, m_ScanlineBuf.size()); + fxcrt::Fill(m_ScanlineBuf, 0xff); if (m_Encoding < 0) { FaxG4GetRow(m_SrcSpan.data(), bitsize, &m_bitpos, m_ScanlineBuf.data(), m_RefBuf, m_OrigWidth); @@ -642,7 +643,7 @@ int bitpos = starting_bitpos; for (int iRow = 0; iRow < height; ++iRow) { uint8_t* line_buf = dest_buf + iRow * pitch; - memset(line_buf, 0xff, pitch); + FXSYS_memset(line_buf, 0xff, pitch); FaxG4GetRow(src_buf, src_size << 3, &bitpos, line_buf, ref_buf, width); FXSYS_memcpy(ref_buf.data(), line_buf, pitch); }
diff --git a/core/fxcodec/flate/flatemodule.cpp b/core/fxcodec/flate/flatemodule.cpp index 50bc1ba..52ec687 100644 --- a/core/fxcodec/flate/flatemodule.cpp +++ b/core/fxcodec/flate/flatemodule.cpp
@@ -103,7 +103,7 @@ uint32_t written = post_pos - pre_pos; if (written < dest_size) - memset(dest_buf + written, '\0', dest_size - written); + FXSYS_memset(dest_buf + written, '\0', dest_size - written); return ret; }
diff --git a/core/fxcodec/jbig2/JBig2_Image.cpp b/core/fxcodec/jbig2/JBig2_Image.cpp index 00ea29f..0b4fc75 100644 --- a/core/fxcodec/jbig2/JBig2_Image.cpp +++ b/core/fxcodec/jbig2/JBig2_Image.cpp
@@ -150,7 +150,7 @@ const uint8_t* pSrc = GetLine(hFrom); if (!pSrc) { - memset(pDst, 0, m_nStride); + FXSYS_memset(pDst, 0, m_nStride); return; } FXSYS_memcpy(pDst, pSrc, m_nStride); @@ -160,7 +160,7 @@ if (!m_pData) return; - memset(data(), v ? 0xff : 0, Fx2DSizeOrDie(m_nStride, m_nHeight)); + FXSYS_memset(data(), v ? 0xff : 0, Fx2DSizeOrDie(m_nStride, m_nHeight)); } bool CJBig2_Image::ComposeTo(CJBig2_Image* pDst, @@ -268,7 +268,8 @@ FX_Alloc(uint8_t, desired_size))); FXSYS_memcpy(data(), pExternalBuffer, current_size); } - memset(data() + current_size, v ? 0xff : 0, desired_size - current_size); + FXSYS_memset(data() + current_size, v ? 0xff : 0, + desired_size - current_size); m_nHeight = h; }
diff --git a/core/fxcodec/progressive_decoder.cpp b/core/fxcodec/progressive_decoder.cpp index e7af998..67420c8 100644 --- a/core/fxcodec/progressive_decoder.cpp +++ b/core/fxcodec/progressive_decoder.cpp
@@ -422,7 +422,7 @@ case 3: { uint8_t gray = FXRGB2GRAY(FXARGB_R(argb), FXARGB_G(argb), FXARGB_B(argb)); - memset(pScanline, gray, sizeX); + FXSYS_memset(pScanline, gray, sizeX); break; } case 8: { @@ -1797,7 +1797,7 @@ pPixelWeights->m_Weights[j - pPixelWeights->m_SrcStart]; dest_g += pixel_weight * src_scan[j]; } - memset(dest_scan, CStretchEngine::PixelFromFixed(dest_g), 3); + FXSYS_memset(dest_scan, CStretchEngine::PixelFromFixed(dest_g), 3); dest_scan += dest_bytes_per_pixel; break; }
diff --git a/core/fxcodec/tiff/tiff_decoder.cpp b/core/fxcodec/tiff/tiff_decoder.cpp index c117a89..ae55fba 100644 --- a/core/fxcodec/tiff/tiff_decoder.cpp +++ b/core/fxcodec/tiff/tiff_decoder.cpp
@@ -107,7 +107,7 @@ } void _TIFFmemset(void* ptr, int val, tmsize_t size) { - memset(ptr, val, static_cast<size_t>(size)); + FXSYS_memset(ptr, val, static_cast<size_t>(size)); } void _TIFFmemcpy(void* des, const void* src, tmsize_t size) {
diff --git a/core/fxcrt/bytestring.cpp b/core/fxcrt/bytestring.cpp index ab54614..e9925f7 100644 --- a/core/fxcrt/bytestring.cpp +++ b/core/fxcrt/bytestring.cpp
@@ -69,9 +69,10 @@ // Span's lifetime must end before ReleaseBuffer() below. pdfium::span<char> buf = ret.GetBuffer(nMaxLen); - // In the following two calls, there's always space in the buffer for - // a terminating NUL that's not included in nMaxLen. - memset(buf.data(), 0, nMaxLen + 1); + // SAFETY: In the following two calls, there's always space in the buffer + // for a terminating NUL that's not included in nMaxLen, and hence not + // included in the span. + UNSAFE_BUFFERS(FXSYS_memset(buf.data(), 0, nMaxLen + 1)); va_copy(argListCopy, argList); vsnprintf(buf.data(), nMaxLen + 1, pFormat, argListCopy); va_end(argListCopy);
diff --git a/core/fxcrt/widestring.cpp b/core/fxcrt/widestring.cpp index 56dbece..944cb82 100644 --- a/core/fxcrt/widestring.cpp +++ b/core/fxcrt/widestring.cpp
@@ -267,15 +267,15 @@ // Span's lifetime must end before ReleaseBuffer() below. pdfium::span<wchar_t> buffer = str.GetBuffer(size); - // In the following two calls, there's always space in the WideString - // for a terminating NUL that's not included in the span. + // SAFETY: In the following two calls, there's always space in the + // WideString for a terminating NUL that's not included in the span. // For vswprintf(), MSAN won't untaint the buffer on a truncated write's // -1 return code even though the buffer is written. Probably just as well // not to trust the vendor's implementation to write anything anyways. // See https://crbug.com/705912. - memset(buffer.data(), 0, (size + 1) * sizeof(wchar_t)); + UNSAFE_BUFFERS( + FXSYS_memset(buffer.data(), 0, (size + 1) * sizeof(wchar_t))); int ret = vswprintf(buffer.data(), size + 1, pFormat, argList); - bool bSufficientBuffer = ret >= 0 || buffer[size - 1] == 0; if (!bSufficientBuffer) return std::nullopt;
diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp index 2d47a7b..b21aafc 100644 --- a/core/fxge/dib/cfx_dibbase.cpp +++ b/core/fxge/dib/cfx_dibbase.cpp
@@ -359,7 +359,7 @@ for (int col = src_left; col < src_left + width; ++col) { uint8_t value = (src_scan[col / 8] & (1 << (7 - col % 8))) ? kSetGray : kResetGray; - memset(dest_scan, value, 3); + FXSYS_memset(dest_scan, value, 3); dest_scan += comps; } } @@ -380,7 +380,7 @@ const uint8_t* src_scan = pSrcBitmap->GetScanline(src_top + row).subspan(src_left).data(); for (int col = 0; col < width; ++col) { - memset(dest_scan, *src_scan, 3); + FXSYS_memset(dest_scan, *src_scan, 3); dest_scan += comps; ++src_scan; } @@ -907,7 +907,7 @@ continue; } if (GetBppFromFormat(m_Format) == 1) { - memset(dest_scan, 0, m_Pitch); + FXSYS_memset(dest_scan, 0, m_Pitch); for (int col = 0; col < m_Width; ++col) { if (src_scan[col / 8] & (1 << (7 - col % 8))) { int dest_col = m_Width - col - 1;
diff --git a/core/fxge/dib/cfx_dibitmap.cpp b/core/fxge/dib/cfx_dibitmap.cpp index b9e5eef..30420b4 100644 --- a/core/fxge/dib/cfx_dibitmap.cpp +++ b/core/fxge/dib/cfx_dibitmap.cpp
@@ -146,19 +146,20 @@ uint8_t* pBuffer = m_pBuffer.Get(); switch (GetFormat()) { case FXDIB_Format::k1bppMask: - memset(pBuffer, (color & 0xff000000) ? 0xff : 0, m_Pitch * m_Height); + FXSYS_memset(pBuffer, (color & 0xff000000) ? 0xff : 0, + m_Pitch * m_Height); break; case FXDIB_Format::k1bppRgb: { int index = FindPalette(color); - memset(pBuffer, index ? 0xff : 0, m_Pitch * m_Height); + FXSYS_memset(pBuffer, index ? 0xff : 0, m_Pitch * m_Height); break; } case FXDIB_Format::k8bppMask: - memset(pBuffer, color >> 24, m_Pitch * m_Height); + FXSYS_memset(pBuffer, color >> 24, m_Pitch * m_Height); break; case FXDIB_Format::k8bppRgb: { int index = FindPalette(color); - memset(pBuffer, index, m_Pitch * m_Height); + FXSYS_memset(pBuffer, index, m_Pitch * m_Height); break; } case FXDIB_Format::kRgb: { @@ -168,7 +169,7 @@ int b; std::tie(a, r, g, b) = ArgbDecode(color); if (r == g && g == b) { - memset(pBuffer, r, m_Pitch * m_Height); + FXSYS_memset(pBuffer, r, m_Pitch * m_Height); } else { int byte_pos = 0; for (int col = 0; col < m_Width; col++) { @@ -722,7 +723,7 @@ for (int row = rect.top; row < rect.bottom; row++) { uint8_t* dest_scan = m_pBuffer.Get() + row * m_Pitch + rect.left; if (src_alpha == 255) { - memset(dest_scan, gray, width); + FXSYS_memset(dest_scan, gray, width); } else { for (int col = 0; col < width; col++) { *dest_scan = FXDIB_ALPHA_MERGE(*dest_scan, gray, src_alpha); @@ -753,7 +754,7 @@ uint8_t left_flag = *dest_scan_top & (255 << (8 - left_shift)); uint8_t right_flag = *dest_scan_top_r & (255 >> right_shift); if (new_width) { - memset(dest_scan_top + 1, index ? 255 : 0, new_width - 1); + FXSYS_memset(dest_scan_top + 1, index ? 255 : 0, new_width - 1); if (!index) { *dest_scan_top &= left_flag; *dest_scan_top_r &= right_flag; @@ -869,7 +870,7 @@ return false; if (dest_format == FXDIB_Format::kArgb) { - memset(dest_buf.get(), 0xff, dest_buf_size); + FXSYS_memset(dest_buf.get(), 0xff, dest_buf_size); } RetainPtr<CFX_DIBBase> holder(this); // SAFETY: `dest_buf` allocated with `dest_buf_size` bytes above.
diff --git a/core/fxge/dib/cfx_scanlinecompositor.cpp b/core/fxge/dib/cfx_scanlinecompositor.cpp index 9c281b8..a85b8b1 100644 --- a/core/fxge/dib/cfx_scanlinecompositor.cpp +++ b/core/fxge/dib/cfx_scanlinecompositor.cpp
@@ -151,7 +151,7 @@ uint8_t* dest_scan = dest_span.data(); const uint8_t* clip_scan = clip_span.data(); if (!clip_scan) { - memset(dest_scan, 0xff, width); + FXSYS_memset(dest_scan, 0xff, width); return; } for (int i = 0; i < width; ++i) {
diff --git a/fpdfsdk/cpdfsdk_helpers_unittest.cpp b/fpdfsdk/cpdfsdk_helpers_unittest.cpp index ac1320e..c2a42a3 100644 --- a/fpdfsdk/cpdfsdk_helpers_unittest.cpp +++ b/fpdfsdk/cpdfsdk_helpers_unittest.cpp
@@ -4,6 +4,8 @@ #include "fpdfsdk/cpdfsdk_helpers.h" +#include "core/fxcrt/compiler_specific.h" +#include "core/fxcrt/fx_memcpy_wrappers.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -21,7 +23,8 @@ // Buffer should not change if declared length is too short. char buf[kExpectedToBeCopiedLen + 1]; - memset(buf, 0x42, kExpectedToBeCopiedLen + 1); + // TODO(tsepez): convert to span. + UNSAFE_BUFFERS(FXSYS_memset(buf, 0x42, kExpectedToBeCopiedLen + 1)); ASSERT_EQ(kExpectedToBeCopiedLen + 1, NulTerminateMaybeCopyAndReturnLength(to_be_copied, buf, kExpectedToBeCopiedLen));
diff --git a/testing/embedder_test.cpp b/testing/embedder_test.cpp index 465105e..7594125 100644 --- a/testing/embedder_test.cpp +++ b/testing/embedder_test.cpp
@@ -275,7 +275,7 @@ void EmbedderTest::SetUp() { UNSUPPORT_INFO* info = static_cast<UNSUPPORT_INFO*>(this); - memset(info, 0, sizeof(UNSUPPORT_INFO)); + FXSYS_memset(info, 0, sizeof(UNSUPPORT_INFO)); info->version = 1; info->FSDK_UnSupport_Handler = UnsupportedHandlerTrampoline; FSDK_SetUnSpObjProcessHandler(info); @@ -343,7 +343,7 @@ EXPECT_TRUE(!loader_); loader_ = std::make_unique<TestLoader>(file_contents_); - memset(&file_access_, 0, sizeof(file_access_)); + FXSYS_memset(&file_access_, 0, sizeof(file_access_)); file_access_.m_FileLen = pdfium::checked_cast<unsigned long>(file_contents_.size()); file_access_.m_GetBlock = TestLoader::GetBlock; @@ -428,7 +428,7 @@ document_.reset(); avail_.reset(); fake_file_access_.reset(); - memset(&file_access_, 0, sizeof(file_access_)); + FXSYS_memset(&file_access_, 0, sizeof(file_access_)); loader_.reset(); file_contents_ = {}; } @@ -437,12 +437,12 @@ FPDF_DOCUMENT doc, JavaScriptOption javascript_option) { IPDF_JSPLATFORM* platform = static_cast<IPDF_JSPLATFORM*>(this); - memset(platform, '\0', sizeof(IPDF_JSPLATFORM)); + FXSYS_memset(platform, '\0', sizeof(IPDF_JSPLATFORM)); platform->version = 3; platform->app_alert = AlertTrampoline; FPDF_FORMFILLINFO* formfillinfo = static_cast<FPDF_FORMFILLINFO*>(this); - memset(formfillinfo, 0, sizeof(FPDF_FORMFILLINFO)); + FXSYS_memset(formfillinfo, 0, sizeof(FPDF_FORMFILLINFO)); formfillinfo->version = form_fill_info_version_; formfillinfo->FFI_Invalidate = InvalidateStub; formfillinfo->FFI_OutputSelectedRect = OutputSelectedRectStub; @@ -686,7 +686,7 @@ FPDF_DOCUMENT EmbedderTest::OpenSavedDocumentWithPassword( const char* password) { - memset(&saved_file_access_, 0, sizeof(saved_file_access_)); + FXSYS_memset(&saved_file_access_, 0, sizeof(saved_file_access_)); saved_file_access_.m_FileLen = pdfium::checked_cast<unsigned long>(data_string_.size()); saved_file_access_.m_GetBlock = GetBlockFromString;
diff --git a/xfa/fgas/font/cfgas_fontmgr.cpp b/xfa/fgas/font/cfgas_fontmgr.cpp index c3a347e..86ddd78 100644 --- a/xfa/fgas/font/cfgas_fontmgr.cpp +++ b/xfa/fgas/font/cfgas_fontmgr.cpp
@@ -521,7 +521,7 @@ // https://bugs.chromium.org/p/pdfium/issues/detail?id=690 FXFT_StreamRec* ftStream = static_cast<FXFT_StreamRec*>(ft_scalloc(sizeof(FXFT_StreamRec), 1)); - memset(ftStream, 0, sizeof(FXFT_StreamRec)); + FXSYS_memset(ftStream, 0, sizeof(FXFT_StreamRec)); ftStream->base = nullptr; ftStream->descriptor.pointer = static_cast<void*>(pFontStream.Get()); ftStream->pos = 0;