commit 607c3faa12336f3194d181cd9722ac88c60c4d45
author Lei Zhang <thestig@chromium.org> Wed Nov 01 19:45:54 2023 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Nov 01 19:45:54 2023 +0000
tree bcaafbc004288febfb370f31fefb51d7180259ea
parent 90589d465a0e51bb87469b9ff1d7480b2454a2ba

Fix potential leak in CGdiplusExt

CGdiplusExt::Load() currently forgets to call FreeLibrary() when
GetProcAddress() fails. Fix this and encapsulate the member variables.
Along the way, rename the member variables to have more meaningful
names, and so they follow the style guide.

Bug: pdfium:1876
Change-Id: I415ec9ccb348b30c5749d297244cae3119b862ea
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/113290
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>

core/fxge/win32/cgdi_plus_ext.cpp

diff --git a/core/fxge/win32/cgdi_plus_ext.cpp b/core/fxge/win32/cgdi_plus_ext.cpp
index 3c33ea0..d001391 100644
--- a/core/fxge/win32/cgdi_plus_ext.cpp
+++ b/core/fxge/win32/cgdi_plus_ext.cpp
@@ -182,7 +182,7 @@
     decltype(&Gdiplus::DllExports::GdipSetPixelOffsetMode);
 #define CallFunc(funcname)               \
   reinterpret_cast<FuncType_##funcname>( \
-      GdiplusExt.m_Functions[FuncId_##funcname])
+      GdiplusExt.functions()[FuncId_##funcname])
 
 Gdiplus::GpFillMode FillType2Gdip(CFX_FillRenderOptions::FillType fill_type) {
   return fill_type == CFX_FillRenderOptions::FillType::kEvenOdd
@@ -557,7 +557,7 @@
 CGdiplusExt::CGdiplusExt() = default;
 
 CGdiplusExt::~CGdiplusExt() {
-  FreeLibrary(m_hModule);
+  FreeLibrary(gdiplus_module_);
 }
 
 void CGdiplusExt::Load() {
@@ -565,22 +565,24 @@
   GetSystemDirectoryA(buf, MAX_PATH);
   ByteString dllpath = buf;
   dllpath += "\\GDIPLUS.DLL";
-  m_hModule = LoadLibraryA(dllpath.c_str());
-  if (!m_hModule)
+  gdiplus_module_ = LoadLibraryA(dllpath.c_str());
+  if (!gdiplus_module_) {
     return;
+  }
 
-  m_Functions.resize(std::size(g_GdipFuncNames));
+  gdiplus_functions_.resize(std::size(g_GdipFuncNames));
   for (size_t i = 0; i < std::size(g_GdipFuncNames); ++i) {
-    m_Functions[i] = GetProcAddress(m_hModule, g_GdipFuncNames[i]);
-    if (!m_Functions[i]) {
-      m_hModule = nullptr;
+    gdiplus_functions_[i] = GetProcAddress(gdiplus_module_, g_GdipFuncNames[i]);
+    if (!gdiplus_functions_[i]) {
+      FreeLibrary(gdiplus_module_);
+      gdiplus_module_ = nullptr;
       return;
     }
   }
 
   ULONG_PTR gdiplus_token;
   Gdiplus::GdiplusStartupInput gdiplus_startup_input;
-  ((FuncType_GdiplusStartup)m_Functions[FuncId_GdiplusStartup])(
+  ((FuncType_GdiplusStartup)gdiplus_functions_[FuncId_GdiplusStartup])(
       &gdiplus_token, &gdiplus_startup_input, nullptr);
 }
 

core/fxge/win32/cgdi_plus_ext.h

diff --git a/core/fxge/win32/cgdi_plus_ext.h b/core/fxge/win32/cgdi_plus_ext.h
index e6a1363..6a93179 100644
--- a/core/fxge/win32/cgdi_plus_ext.h
+++ b/core/fxge/win32/cgdi_plus_ext.h
@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "core/fxcrt/retain_ptr.h"
+#include "third_party/base/containers/span.h"
 
 class CFX_DIBBase;
 class CFX_GraphStateData;
@@ -28,7 +29,7 @@
   ~CGdiplusExt();
 
   void Load();
-  bool IsAvailable() { return !!m_hModule; }
+  bool IsAvailable() { return !!gdiplus_module_; }
   bool StretchDIBits(HDC hDC,
                      const RetainPtr<CFX_DIBBase>& source,
                      int dest_left,
@@ -45,10 +46,11 @@
                 uint32_t stroke_argb,
                 const CFX_FillRenderOptions& fill_options);
 
-  std::vector<FARPROC> m_Functions;
+  pdfium::span<const FARPROC> functions() const { return gdiplus_functions_; }
 
  private:
-  HMODULE m_hModule = nullptr;
+  HMODULE gdiplus_module_ = nullptr;
+  std::vector<FARPROC> gdiplus_functions_;
 };
 
 #endif  // CORE_FXGE_WIN32_CGDI_PLUS_EXT_H_