Windows - Create the Khronos\OpenXR\1\ApiLayers\Implicit if it doesn't exist

[Louis-Billot]

Hello,

Thank you for your great tool, it helps me a lot when managing my layers!

Recently, I had to install a layer on a computer that never had one installed and this app could not create the registry key.

This little pull-request allows the app to create the registry key if it is not already there.

Maybe the original behavior of the app was the one intended (for security reasons?) and in this case fell free to discard this PR.

Don't hesitate to contact me if you want more informations, even though I think the changes are quite minimal

[fredemmott]

Thanks, this looks good! I'd prefer nullptr over NULL for pointer params, but my only real concern is what ACL is set if this creates the HKCU key - in particular, an unprivileged regedit should be able to alter that key and any values in it, even though this app is elevated.

I'll merge later after testing that if all seems good

[Louis-Billot]

I understand your concern but sadly I'm not familiar enough with the rights of registry keys to provide you a solution/answer.

I just added a new commit switching from NULL to nullptr, it compiled fine!

I faced this 'bug' and it seemed it was something 'easily' fixable so I checked the code and made this small PR but I'm in no way an expert.
Feel free to further modify the branch of my fork (the box 'Allow edits by maintainers' of this PR is checked)

[Louis-Billot]

Another thing (this may not be the best place to post it)

When cloning your repo, I first tried compiling it before adding my changes but faced a compilation error

C:\Users\Louis\Documents\git\OpenXR-API-Layers-GUI\src\windows\GetKnownFolderPath.hpp(30,5): error C2593: 'operator =' is ambiguous [C:\Users\Louis\Documents\git\OpenXR-API-Layers-GUI\build\src\lib.vcxproj]
[build]       path = {std::wstring_view {buf}};
[build]       ^ (compiling source file '../../src/windows/WindowsAPILayerStore.cpp')
[build]   C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include\filesystem(681,15):
[build]   could be 'std::filesystem::path &std::filesystem::path::operator =(std::filesystem::path::string_type &&) noexcept'
[build]           path& operator=(string_type&& _Source) noexcept /* strengthened */ {
[build]                 ^
[build]   C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.38.33130\include\filesystem(646,15):
[build]   or       'std::filesystem::path &std::filesystem::path::operator =(std::filesystem::path &&) noexcept'
[build]           path& operator=(path&&) noexcept = default;

I suppose it is an error only occuring on my setup (no idea why) and I solved it by changing GetKnownFolderPath.hpp(30,5) like this:

from
path = {std::wstring_view {buf}};
to
path = std::filesystem::path {std::wstring_view {buf}};

It is probably not the correct way to do it, either way it was just to get the app to compile on my machine.

[Louis-Billot]

I understand your concern but sadly I'm not familiar enough with the rights of registry keys to provide you a solution/answer.

In fact, by reading the documentation of RegCreateKeyExW a bit more carefully, it seems that the ACL should be inherited from the direct parent (which I think is the desired behavior):

[in, optional] lpSecurityAttributes

A pointer to a SECURITY_ATTRIBUTES structure that determines whether the returned handle can be inherited by child processes. If lpSecurityAttributes is NULL, the handle cannot be inherited.

The lpSecurityDescriptor member of the structure specifies a security descriptor for the new key. If lpSecurityAttributes is NULL, the key gets a default security descriptor. The ACLs in a default security descriptor for a key are inherited from its direct parent key.

[fredemmott]

Another thing (this may not be the best place to post it)

When cloning your repo, I first tried compiling it before adding my changes but faced a compilation error

from
path = {std::wstring_view {buf}};
to
path = std::filesystem::path {std::wstring_view {buf}};

It is probably not the correct way to do it, either way it was just to get the app to compile on my machine.

MSVC's pickiness here varies and improves fairly rapidly; AIUI your change is correct, but should be unneeded. The likely cause is that your MSVC C++ toolset is slightly out of date - you're using 14.38, but as of May, the current version is 14.30 - there have also been several bugfixes to MSVC's C++ in more recent updates. Updating visual studio should resolve the issue.

[fredemmott]

Huh, weird, the CI build crashes on startup, but if I build the branch myself it's fine.

[Louis-Billot]

Weird, from my side I see a "box" on this PR saying 'all checks have passed' and in the details I can see the compilation worked.
Isn't that the CI ?

[fredemmott]

Yep, but if I download the result and try to run it, it crashes. On the plus side, it leaves a dump, but unfortunately I didn't have the foresight to upload the debug symbols to CI, so the dumps aren't really useful >_<

[Louis-Billot]

Oh ok, I thought you meant the compilation crashing 😅

[fredemmott]

Huhh... very weird. SHGetKnownFolderPath(FOLDERID_ProgramFilesX64) is failing with ERROR_FILE_NOT_FOUND

Possible difference is because I'm doing a 64-bit build locally but the remote build is 32-bit - but the 'X64' FOLDERID is supposed to work regardless of bitness.

This ends up being an uncaught hresult_exception

The really weird thing is what ends up in the crash report as the cause of process termination:

ERROR_CODE: (NTSTATUS) 0xc0000409 - The system detected an overrun of a stack-based buffer in this application. This overrun could potentially allow a malicious user to gain control of this application.

[fredemmott]

Merging, same issue exists in main, not caused by this PR.

Another possible cause is that right now, my local build is with clang rather than MSVC (because it gives much better error messages for template problems)

[fredemmott]

Thanks again, and sorry for the noise :)

[Louis-Billot]

No problem, thank you for the great app !