dotnet / dotnet/deployment-tools

NetCoreCheck: access violation when failed to open runtimeconfig.json for writing

Open
#471 1 comment 0 reactions 1 assignee Claimed by @MSLukeWest View on GitHub
area-NETCoreCheck
Dominant language
C#
Stars
213
Forks
73
Avg merge
1d 21h
Merged PRs (30d)
4

Description

### Description

Access violation occurs if NetCoreCheck.exe fails to open a Temp Runtime Config File `TestXXX.runtimeconfig.json` for writing. This is reproducible with file version 1.0.2.0 from [netcorecheck x64 7.0.0](https://www.nuget.org/packages/Microsoft.NET.Tools.NETCoreCheck.x64) and seems to be reproducible with the current main 1478b7e6938013629e182b0656bfbde04d367cc5.

To reproduce, place NetCoreCheck.exe to a folder where the current user can't create files, e.g. forbidden by NTFS security. The current working folder is not important. Run the command:

```
Path\to\Restricted\Folder\NetCoreCheck.exe --runtimename Microsoft.WindowsDesktop.App --runtimeversion 8.0.15
```

Expected behavior: this is tricky because thanks to this crash I found a bug in an installer wizard. If from now we just print an error and exit then a program that just checks the exit status would think that the runtime version is not installed. So maybe print the error and crash anyway with throw or `std::abort()`.

Actual: Access Violation ([crash dialog may be disabled in group policy](https://superuser.com/a/1349113/233620), but an error entry will be created in the Windows Application Log),

Clearly, there's a bug in a printf-like call where DWORD value is passed in place of a string conversion specifier %s.

https://github.com/dotnet/deployment-tools/blob/1478b7e6938013629e182b0656bfbde04d367cc5/src/clickonce/native/projects/NetCoreCheck/TempRuntimeConfigFile.cpp#L84

I also think that the Log function can be marked as printf-like to avoid such problems in the future:
```
#include // for _Printf_format_string_

class Logger
{
public:
virtual void Log(_Printf_format_string_ LPCWSTR pszFormat, ...) const noexcept = 0;
...
```

It's unclear what the correct error message display: the error code, the path, or both.

This was found in an installer wizard that creates a temp folder and unpacks NetCoreCheck.exe in an elevated mode and with NTFS inheritance disabled and runs it in a regular mode.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.