Prevent crashes in win32 GDI code due to null bitmap pointers This CL adds CFX_DIBBase::RealizeIfNeeded() as the spiritial successor to CFX_DIBExtractor, which https://pdfium-review.googlesource.com/108371 removed. This is necessary for interactions with some GDI APIs that do not operate on scanlines and require a pointer to a contiguous bitmap buffer. Call RealizeIfNeeded() closer to where the GDI API calls happen, instead of exactly where the prior CFX_DIBExtractor uses were. With the RealizeIfNeeded() calls in place, the GDI APIs will no longer receive null buffer pointers and crash. Unlike CFX_DIBExtractor, RealizeIfNeeded() won't create a new CFX_DIBitmap when an existing CFX_DIBBase already has a contiguous buffer. Bug: chromium:1466498 Change-Id: I33b46a79c2f0a2ad467e40171db328c4f74b8153 Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/111433 Commit-Queue: Nigi <nigi@chromium.org> Reviewed-by: Nigi <nigi@chromium.org>
diff --git a/core/fxge/dib/cfx_dibbase.cpp b/core/fxge/dib/cfx_dibbase.cpp index b64bc28..4aa586f 100644 --- a/core/fxge/dib/cfx_dibbase.cpp +++ b/core/fxge/dib/cfx_dibbase.cpp
@@ -633,6 +633,15 @@ return GetRequiredPaletteSize() * sizeof(uint32_t); } +#if BUILDFLAG(IS_WIN) +RetainPtr<const CFX_DIBBase> CFX_DIBBase::RealizeIfNeeded() const { + if (GetBuffer().empty()) { + return Realize(); + } + return pdfium::WrapRetain(this); +} +#endif + RetainPtr<CFX_DIBitmap> CFX_DIBBase::Realize() const { return ClipToInternal(nullptr); }
diff --git a/core/fxge/dib/cfx_dibbase.h b/core/fxge/dib/cfx_dibbase.h index ebf1eac..6327888 100644 --- a/core/fxge/dib/cfx_dibbase.h +++ b/core/fxge/dib/cfx_dibbase.h
@@ -9,6 +9,7 @@ #include <stdint.h> +#include "build/build_config.h" #include "core/fxcrt/data_vector.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxge/dib/fx_dib.h" @@ -64,6 +65,11 @@ // Copies into internally-owned palette. void SetPalette(pdfium::span<const uint32_t> src_palette); +#if BUILDFLAG(IS_WIN) + // Calls Realize() if needed. Otherwise, return `this`. + RetainPtr<const CFX_DIBBase> RealizeIfNeeded() const; +#endif + RetainPtr<CFX_DIBitmap> Realize() const; RetainPtr<CFX_DIBitmap> ClipTo(const FX_RECT& rect) const; RetainPtr<CFX_DIBitmap> ConvertTo(FXDIB_Format format) const;
diff --git a/core/fxge/win32/cgdi_device_driver.cpp b/core/fxge/win32/cgdi_device_driver.cpp index 6ae582e..aa1d13f 100644 --- a/core/fxge/win32/cgdi_device_driver.cpp +++ b/core/fxge/win32/cgdi_device_driver.cpp
@@ -153,7 +153,7 @@ EndPath(hDC); } -ByteString GetBitmapInfo(const RetainPtr<CFX_DIBBase>& source) { +ByteString GetBitmapInfo(const RetainPtr<const CFX_DIBBase>& source) { int len = sizeof(BITMAPINFOHEADER); if (source->GetBPP() == 1 || source->GetBPP() == 8) { len += sizeof(DWORD) * (int)(1 << source->GetBPP()); @@ -399,24 +399,34 @@ return false; } - ByteString info = GetBitmapInfo(flipped_source); + RetainPtr<const CFX_DIBBase> realized_source = + flipped_source->RealizeIfNeeded(); + if (!realized_source) { + return false; + } + ByteString info = GetBitmapInfo(realized_source); ((BITMAPINFOHEADER*)info.c_str())->biHeight *= -1; FX_RECT dst_rect(0, 0, src_rect.Width(), src_rect.Height()); - dst_rect.Intersect(0, 0, flipped_source->GetWidth(), - flipped_source->GetHeight()); + dst_rect.Intersect(0, 0, realized_source->GetWidth(), + realized_source->GetHeight()); int dst_width = dst_rect.Width(); int dst_height = dst_rect.Height(); ::StretchDIBits(m_hDC, left, top, dst_width, dst_height, 0, 0, dst_width, - dst_height, flipped_source->GetBuffer().data(), + dst_height, realized_source->GetBuffer().data(), (BITMAPINFO*)info.c_str(), DIB_RGB_COLORS, SRCCOPY); return true; } - ByteString info = GetBitmapInfo(source); - ::SetDIBitsToDevice(m_hDC, left, top, src_rect.Width(), src_rect.Height(), - src_rect.left, source->GetHeight() - src_rect.bottom, 0, - source->GetHeight(), source->GetBuffer().data(), - (BITMAPINFO*)info.c_str(), DIB_RGB_COLORS); + RetainPtr<const CFX_DIBBase> realized_source = source->RealizeIfNeeded(); + if (!realized_source) { + return false; + } + ByteString info = GetBitmapInfo(realized_source); + ::SetDIBitsToDevice( + m_hDC, left, top, src_rect.Width(), src_rect.Height(), src_rect.left, + realized_source->GetHeight() - src_rect.bottom, 0, + realized_source->GetHeight(), realized_source->GetBuffer().data(), + (BITMAPINFO*)info.c_str(), DIB_RGB_COLORS); return true; } @@ -430,7 +440,6 @@ return false; } - ByteString info = GetBitmapInfo(source); if ((int64_t)abs(dest_width) * abs(dest_height) < (int64_t)source->GetWidth() * source->GetHeight() * 4 || options.bInterpolateBilinear) { @@ -438,21 +447,29 @@ } else { SetStretchBltMode(m_hDC, COLORONCOLOR); } - RetainPtr<CFX_DIBBase> stretch_source = source; + + RetainPtr<const CFX_DIBBase> realized_source; if (m_DeviceType == DeviceType::kPrinter && ((int64_t)source->GetWidth() * source->GetHeight() > (int64_t)abs(dest_width) * abs(dest_height))) { - stretch_source = source->StretchTo(dest_width, dest_height, - FXDIB_ResampleOptions(), nullptr); + RetainPtr<CFX_DIBBase> stretch_source = source->StretchTo( + dest_width, dest_height, FXDIB_ResampleOptions(), nullptr); if (!stretch_source) { return false; } - info = GetBitmapInfo(stretch_source); + realized_source = stretch_source->RealizeIfNeeded(); + } else { + realized_source = source->RealizeIfNeeded(); } + if (!realized_source) { + return false; + } + + ByteString info = GetBitmapInfo(realized_source); ::StretchDIBits(m_hDC, dest_left, dest_top, dest_width, dest_height, 0, 0, - stretch_source->GetWidth(), stretch_source->GetHeight(), - stretch_source->GetBuffer().data(), (BITMAPINFO*)info.c_str(), - DIB_RGB_COLORS, SRCCOPY); + realized_source->GetWidth(), realized_source->GetHeight(), + realized_source->GetBuffer().data(), + (BITMAPINFO*)info.c_str(), DIB_RGB_COLORS, SRCCOPY); return true; } @@ -466,8 +483,13 @@ return false; } - int width = source->GetWidth(); - int height = source->GetHeight(); + RetainPtr<const CFX_DIBBase> realized_source = source->RealizeIfNeeded(); + if (!realized_source) { + return false; + } + + int width = realized_source->GetWidth(); + int height = realized_source->GetHeight(); struct { BITMAPINFOHEADER bmiHeader; uint32_t bmiColors[2]; @@ -507,8 +529,8 @@ // 0xB8074A ::StretchDIBits(m_hDC, dest_left, dest_top, dest_width, dest_height, 0, 0, - width, height, source->GetBuffer().data(), (BITMAPINFO*)&bmi, - DIB_RGB_COLORS, 0xB8074A); + width, height, realized_source->GetBuffer().data(), + (BITMAPINFO*)&bmi, DIB_RGB_COLORS, 0xB8074A); SelectObject(m_hDC, hOld); DeleteObject(hPattern);
diff --git a/core/fxge/win32/cgdi_plus_ext.cpp b/core/fxge/win32/cgdi_plus_ext.cpp index 42cf505..f6a9181 100644 --- a/core/fxge/win32/cgdi_plus_ext.cpp +++ b/core/fxge/win32/cgdi_plus_ext.cpp
@@ -222,16 +222,22 @@ dest_height); return; } - int src_pitch = source->GetPitch(); + + RetainPtr<const CFX_DIBBase> realized_source = source->RealizeIfNeeded(); + if (!realized_source) { + return; + } + + int src_pitch = realized_source->GetPitch(); // `GdipCreateBitmapFromScan0()` requires a `BYTE*` scanline buffer, but in // this case, the bitmap only gets read by `GdipDrawImagePointsI()`, then // disposed of, so it's safe to cast away `const` here. - uint8_t* scan0 = - const_cast<uint8_t*>(source->GetBuffer() - .subspan(src_rect.top * src_pitch + - source->GetBPP() * src_rect.left / 8) - .data()); + uint8_t* scan0 = const_cast<uint8_t*>( + realized_source->GetBuffer() + .subspan(src_rect.top * src_pitch + + realized_source->GetBPP() * src_rect.left / 8) + .data()); Gdiplus::GpBitmap* bitmap = nullptr; switch (source->GetFormat()) { case FXDIB_Format::kArgb: @@ -254,7 +260,7 @@ pal[0] = 0; pal[1] = 256; for (int i = 0; i < 256; i++) - pal[i + 2] = source->GetPaletteArgb(i); + pal[i + 2] = realized_source->GetPaletteArgb(i); CallFunc(GdipSetImagePalette)(bitmap, (Gdiplus::ColorPalette*)pal); break; }