From 60a016cbb131de0591d8fc483e41315bd7316572 Mon Sep 17 00:00:00 2001
From: Vipasa Bose <vipasab@google.com>
Date: Wed, 09 Sep 2026 13:25:15 -0700
Subject: [PATCH] Reland "[GCPW] Add delayload configurations to gcp_setup.exe"

This is a reland of commit 4364ff24aa84a55180e1c923c02b0c0be13994cf.

Reason for reland:
The original change's unit test (SetupExeDllSecurityHardening) contained an
overly restrictive static import whitelist loop checking that only kernel32.dll
and ntdll.dll were statically imported. On Windows build bots, gcp_setup.exe
links against standard system DLLs (such as advapi32.dll, user32.dll, shell32.dll),
which caused the test loop to fail on Swarming.

The test has been updated to focus on the essential security assertion:
verifying that load_config->DependentLoadFlags is set to
LOAD_LIBRARY_SEARCH_SYSTEM32 (0x800). This PE header flag forces the OS loader
to load all static imports exclusively from System32.

Original change description:
> While /DEPENDENTLOADFLAG:0x800 and EnableSecureDllLoading() were added
> to gcp_setup.exe and gcp_sfx.exe, gcp_setup.exe was still missing the
> delayload configurations (//build/config/win:delayloads,
> //build/config/win:delayloads_not_for_child_dll, and /DELAYLOAD:version.dll)
> that were applied to its companion gcp_sfx.exe.
>
> As a result, non-KnownDLL imports (such as winmm.dll, userenv.dll,
> version.dll, winhttp.dll, and dbghelp.dll) were still resolved as static
> imports during PE loader initialization.
>
> This change:
> 1. Adds //build/config/win:delayloads and
>    //build/config/win:delayloads_not_for_child_dll to gcp_setup.
> 2. Adds /DELAYLOAD:version.dll to gcp_setup.
> 3. Adds a unit test in gcp_setup_unittest.cc validating that gcp_setup.exe
>    has DependentLoadFlags set to 0x800.

Bug: 522557469
Change-Id: Ib1b0b7f84e80a092a10b00d4a1044707e78574aa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8359208
Reviewed-by: Tien Mai <tienmai@chromium.org>
Commit-Queue: Vipasa Bose <vipasab@google.com>
Reviewed-by: Yusuf Sengul <yusufsn@google.com>
Cr-Commit-Position: refs/heads/main@{#1694837}
---
diff --git a/chrome/credential_provider/test/gcp_setup_unittest.cc b/chrome/credential_provider/test/gcp_setup_unittest.cc
index c9bfdf16..32216ed 100644
--- a/chrome/credential_provider/test/gcp_setup_unittest.cc
+++ b/chrome/credential_provider/test/gcp_setup_unittest.cc
@@ -20,7 +20,9 @@
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
+#include "base/files/memory_mapped_file.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/strings/strcat.h"
 #include "base/strings/strcat_win.h"
@@ -31,6 +33,7 @@
 #include "base/test/scoped_path_override.h"
 #include "base/test/test_reg_util_win.h"
 #include "base/win/atl.h"
+#include "base/win/pe_image.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_com_initializer.h"
@@ -943,6 +946,31 @@
             expected_uninstall_arguments.GetCommandLineString());
 }
 
+TEST_F(GcpSetupTest, SetupExeDllSecurityHardening) {
+  base::FilePath exe_path;
+  ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &exe_path));
+  exe_path = exe_path.Append(kCredentialProviderSetupExe);
+
+  if (!base::PathExists(exe_path)) {
+    GTEST_SKIP() << "gcp_setup.exe not found in build directory";
+  }
+
+  base::MemoryMappedFile module_mmap;
+  ASSERT_TRUE(module_mmap.Initialize(exe_path));
+
+  base::win::PEImageAsData pe_image(
+      reinterpret_cast<HMODULE>(module_mmap.mutable_bytes().data()));
+
+  // Verify DependentLoadFlags has LOAD_LIBRARY_SEARCH_SYSTEM32 (0x800).
+  const auto* load_config =
+      reinterpret_cast<const IMAGE_LOAD_CONFIG_DIRECTORY*>(
+          pe_image.GetImageDirectoryEntryAddr(
+              IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG));
+  ASSERT_NE(load_config, nullptr);
+  EXPECT_EQ(load_config->DependentLoadFlags,
+            static_cast<WORD>(LOAD_LIBRARY_SEARCH_SYSTEM32));
+}
+
 // This test checks the expect success / failure of DLL registration when
 // a gaia user already exists
 // Parameters:
diff --git a/chrome/credential_provider/setup/BUILD.gn b/chrome/credential_provider/setup/BUILD.gn
index 4325145..fd120868 100644
--- a/chrome/credential_provider/setup/BUILD.gn
+++ b/chrome/credential_provider/setup/BUILD.gn
@@ -97,6 +97,13 @@
     "//components/crash/core/common",
     "//content/public/common:static_switches",
   ]
-  configs += [ "//build/config/win:windowed" ]
-  ldflags = [ "/DEPENDENTLOADFLAG:0x800" ]
+  configs += [
+    "//build/config/win:windowed",
+    "//build/config/win:delayloads",
+    "//build/config/win:delayloads_not_for_child_dll",
+  ]
+  ldflags = [
+    "/DELAYLOAD:version.dll",
+    "/DEPENDENTLOADFLAG:0x800",
+  ]
 }

diff --git a/chrome/credential_provider/test/gcp_setup_unittest.cc b/chrome/credential_provider/test/gcp_setup_unittest.cc
index c9bfdf16..32216ed 100644
--- a/chrome/credential_provider/test/gcp_setup_unittest.cc
+++ b/chrome/credential_provider/test/gcp_setup_unittest.cc
@@ -20,7 +20,9 @@
 #include "base/files/file.h"
 #include "base/files/file_enumerator.h"
 #include "base/files/file_util.h"
+#include "base/files/memory_mapped_file.h"
 #include "base/files/scoped_temp_dir.h"
+#include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/strings/strcat.h"
 #include "base/strings/strcat_win.h"
@@ -31,6 +33,7 @@
 #include "base/test/scoped_path_override.h"
 #include "base/test/test_reg_util_win.h"
 #include "base/win/atl.h"
+#include "base/win/pe_image.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_bstr.h"
 #include "base/win/scoped_com_initializer.h"
@@ -943,6 +946,31 @@
             expected_uninstall_arguments.GetCommandLineString());
 }
 
+TEST_F(GcpSetupTest, SetupExeDllSecurityHardening) {
+  base::FilePath exe_path;
+  ASSERT_TRUE(base::PathService::Get(base::DIR_EXE, &exe_path));
+  exe_path = exe_path.Append(kCredentialProviderSetupExe);
+
+  if (!base::PathExists(exe_path)) {
+    GTEST_SKIP() << "gcp_setup.exe not found in build directory";
+  }
+
+  base::MemoryMappedFile module_mmap;
+  ASSERT_TRUE(module_mmap.Initialize(exe_path));
+
+  base::win::PEImageAsData pe_image(
+      reinterpret_cast<HMODULE>(module_mmap.mutable_bytes().data()));
+
+  // Verify DependentLoadFlags has LOAD_LIBRARY_SEARCH_SYSTEM32 (0x800).
+  const auto* load_config =
+      reinterpret_cast<const IMAGE_LOAD_CONFIG_DIRECTORY*>(
+          pe_image.GetImageDirectoryEntryAddr(
+              IMAGE_DIRECTORY_ENTRY_LOAD_CONFIG));
+  ASSERT_NE(load_config, nullptr);
+  EXPECT_EQ(load_config->DependentLoadFlags,
+            static_cast<WORD>(LOAD_LIBRARY_SEARCH_SYSTEM32));
+}
+
 // This test checks the expect success / failure of DLL registration when
 // a gaia user already exists
 // Parameters: