Skip to content

Make Windows system-DLL P/Invokes use only DllImportSearchPath.System32#129588

Open
elinor-fung wants to merge 3 commits into
dotnet:mainfrom
elinor-fung:pinvoke-system32
Open

Make Windows system-DLL P/Invokes use only DllImportSearchPath.System32#129588
elinor-fung wants to merge 3 commits into
dotnet:mainfrom
elinor-fung:pinvoke-system32

Conversation

@elinor-fung

@elinor-fung elinor-fung commented Jun 18, 2026

Copy link
Copy Markdown
Member

Note

This pull request was authored with the assistance of GitHub Copilot.

Adds [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] to every P/Invoke that targets a Windows operating-system library under src/libraries/Common/src/Interop/Windows/ (advapi32, bcrypt, crypt32, kernel32, ntdll, ole32, oleaut32, secur32, shell32, user32, normaliz, ucrtbase, ws2_32, winhttp, the api-set forwarders, etc.).

SPCL and libraries specify DllImportSearchPath.Assembly and DllImportSearchPath.System32. The assembly directory (application directory for single-file) is always searched first. For correctness, we can restrict Windows system DLL p/invokes to System32.

elinor-fung and others added 2 commits June 17, 2026 17:06
Add [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] to every P/Invoke targeting a Windows system DLL under Common/src/Interop/Windows (advapi32, bcrypt, crypt32, kernel32, ntdll, ole32, oleaut32, secur32, shell32, user32, normaliz, ucrtbase, ws2_32, winhttp, etc., plus api-set forwarders).

By default these resolve via the assembly/app directory before System32, which lets a DLL planted next to the application win the search (DLL hijacking). Restricting the search to System32 (LOAD_LIBRARY_SEARCH_SYSTEM32) closes that vector. All targeted DLLs are OS components resident in %windir%\System32. App-local native libraries (hostpolicy, System.Globalization.Native, System.IO.Compression.Native, msquic) live outside these folders and are intentionally left unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…okes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/interop-contrib
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

@elinor-fung elinor-fung changed the title Restrict Windows system-DLL P/Invokes to the System32 search path Make Windows system-DLL P/Invokes use only DllImportSearchPath.System32 Jun 18, 2026
@tannergooding

Copy link
Copy Markdown
Member

I wonder if it is worth discussion of adding some feature for this that doesn't require annotating every p/invoke.

Some thoughts are:

  • An option that requires System32 to be scanned prior to the user folder
  • An option that allows specifying the DefaultDllImportSearchPaths for a given library name: [assembly: DefaultDllImportSearchPaths(DllImportSearchPath.System32, DllName = "Kernel32")]
  • A config switch or environment knob that controls this behavior, with a default populated list of core system binaries
  • etc

CC. @AaronRobinsonMSFT, @dotnet/interop-contrib

-- I don't think it's worth blocking this PR over, but this does seem like a more general problem that would be worth tackling. It would also reduce risk for future P/Invokes added and reduce burden on user binding libraries.

Comment thread docs/coding-guidelines/interop-guidelines.md Outdated
Comment thread docs/coding-guidelines/interop-guidelines.md Outdated
Co-authored-by: Jan Kotas <jkotas@microsoft.com>
Copilot AI review requested due to automatic review settings June 18, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

4 participants