Missing DefaultDllImportSearchPaths attribute in WindowsAppRuntime_EnsureIsLoaded when WindowsAppSDKSelfContained is turned on

[aries-zhang]

Describe the bug

When both WindowsAppSDKSelfContained and code analysis are turned on for a WinUI 3 project, it fails to build due to
CA5392: Use DefaultDllImportSearchPaths attribute for P/Invokes in Microsoft.Windows.Foundation.UndockedRegFreeWinRTCS.WindowsAppRuntime_EnsureIsLoaded.

Steps to reproduce the bug

1. Create a WinUI 3 project (either packaged with single project or with WAP).
2. Add <WindowsAppSDKSelfContained>true</WindowsAppSDKSelfContained> in the project file.
3. Turn on code analysis with the following project properties:

   <AnalysisMode>all</AnalysisMode>
   <AnalysisLevel>latest</AnalysisLevel>
   <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
   <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
   

4. Build the project.

See example code repo here: https://github.com/[aries-zhang/WinUI3-Publish-SelfContained](https://github.com/aries-zhang/WinUI3-Publish-SelfContained)

Expected behavior

1. Build should be successful.
2. No potential DLL hijacking risks should present.

Screenshots

NuGet package version

WinUI 3 - Windows App SDK 1.6.1: 1.6.240923002

Windows version

Windows 11 (24H2): Build 22621

Additional context

Since this is generated code (or included from Nuget?), it makes it hard to suppress the error individually if not a global suppression rule.

[DrusTheAxe]

This warning comes from dev\UndockedRegFreeWinRT\UndockedRegFreeWinRT-AutoInitializer.cs

[DllImport("Microsoft.WindowsAppRuntime.dll", CharSet = CharSet.Unicode, ExactSpelling = true)]
internal static extern int WindowsAppRuntime_EnsureIsLoaded();

@Scottj1s is there a value that could work in all cases? If <WindowsAppSdkUndockedRegFreeWinRTInitialize>true</> is Microsoft.WindowsAppRuntime.dll always colocated with the process' exe? Is WindowsAppSdkUndockedRegFreeWinRTInitialize=true only usable by EXEs or can be used in other cases e.g. class libraries? Would adding this attribute achieve the goal in all cases:

[DefaultDllImportSearchPaths(DllImportSearchPath.ApplicationDirectory)]

Here's the full list of possible values (bitfield, combine as desired): https://learn.microsoft.com/dotnet/api/system.runtime.interopservices.dllimportsearchpath?view=net-8.0#fields

• LegacyBehavior -- is this accurate and better than not specified?
• AssemblyDirectory
• UseDllDirectoryForDependencies
• ApplicationDirectory -- is this right in all cases?
• UserDirectories
• System32 -- shudder. Just say no
• SafeDirectories -- multiple including System32 so, no

[yeelam-gordon]

@DrusTheAxe and @Scottj1s , should we consider for 3 values only, i.e.
[DefaultDllImportSearchPaths(
DllImportSearchPath.AssemblyDirectory |
DllImportSearchPath.SafeDirectories |
DllImportSearchPath.System32)]

Given:

1. AssemblyDirectory: covers the scenario where Microsoft.WindowsAppRuntime.dll is deployed alongside the .NET assemblies (common in WindowsAppSDKSelfContained = true).

2. System32: covers the scenario where the DLL is installed system-wide (common if the user installed the runtime from the MSIX framework package, or used “undocked” mode pointing to a system location).

3. SafeDirectories: includes other protective flags that help guard against DLL hijacking in user directories.

Not for

1. LegacyBehavior: Don't need this given we specify all 3 values in above.

2. UseDllDirectoryForDependencies: That's for dependent DLLs for an app to be in another folder. WinAppSDK should not be there.

3. UserDirectories: Similar for above.

[Scottj1s]

@DrusTheAxe raises a good question with third party libraries implemented over WinAppSDK. The main question I have is whether the DllImportSearchPath enumerators also cover referenced framework package folders (the default location for Microsoft.WindowsAppRuntime.dll, if WindowsAppSdkSelfContained is not set). Some experimentation is needed. It may be best to go with LegacyBehavior if we're only looking to resolve a build break, and not to change behavior.

[MuyuanMS]

@DrusTheAxe @Scottj1s @yeelam-gordon , when I tried some of the values from enum for DefaultDllImportSearchPaths, it gives me another error CA5393

Use of unsafe DllImportSearchPath value ApplicationDirectory
CA5393	There could be a malicious DLL in the default DLL search directories. Or, depending on where your application is run from, there could be a malicious DLL in the application's directory. Use a DllImportSearchPath value that specifies an explicit search path instead. The DllImportSearchPath flags that this rule looks for can be configured in .editorconfig.

From the documentation, only SafeDirectories, System32, UserDirectories are considered safe

[DrusTheAxe]

Use of unsafe DllImportSearchPath value ApplicationDirectory
...
From the documentation, only SafeDirectories, System32, UserDirectories are considered safe

That's weird as SafeDirectories includes the application directory

https://learn.microsoft.com/en-us/dotnet/api/system.runtime.interopservices.dllimportsearchpath?view=net-9.0#fields

Name	Value	Description
ApplicationDirectory	512	Include the application directory in the DLL search path.
UserDirectories	1024	Include any path that was explicitly added to the process-wide search path by using the Win32 AddDllDirectory function.
System32	2048	Include the %WinDir%\System32 directory in the DLL search path.
SafeDirectories	4096	Include the application directory, the %WinDir%\System32 directory, and user directories in the DLL search path.

SafeDirectories == ApplicationDirectory + UserDirectories + System32

Why is ApplicationDirectory unsafe when used explicitly but OK when used as part of SafeDirectories?

You see no CA5393 if you specify SafeDirectories?

DefaultDllImportSearchPaths isn't new (dating back to .NET Core 1.0 and .NET Framework 4.5) but the analyzer rule is? Any known issues with the analyzer? I'm not aware of any, but I don't follow that very closely. Big wide computing world out there, one can't know everything :-)

[MuyuanMS]

@DrusTheAxe

You see no CA5393 if you specify SafeDirectories?

That's right, no CA5393 for SafeDirectories, but for ApplicationDirectory. I don't quite get the logic behind it yet either.
Out of curiosity, from the comment at the beginning of dev/UndockedRegFreeWinRT/UndockedRegFreeWinRT-AutoInitializer.cs, it states to be auto-generated and should be excluded from StyleCop analysis. Do you know whether this statement still apply?

[DrusTheAxe]

I don't quite get the logic behind it yet either.
Tres' bizarre. This smells like a bug in the analyzer/rule. @Scottj1s do you know where these days VS issues should be reported?

Do you know whether this statement still apply?
I haven't heard otherwise (FWIW). I'd expect that would exclude the file from analysis but apparently not, which smells like a VS bug to me.

[Scottj1s]

Note that from a bitwise OR perspective,
SafeDirectories != ApplicationDirectory + UserDirectories + System32

Iow, SafeDirectories is a distinct policy from just combining ApplicationDirectory, UserDirectories, and System32. That may account for the different behavior.

Have we determined to what extent FrameworkPackage locations are supported by DllImportSearchPath values? That's the most important question.

I don't think a generated code opt-out resolves this - the threat remains and must be dealt with.