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;
}
