Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve DLL loading behavior on Windows #11569

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

notr1ch
Copy link
Member

@notr1ch notr1ch commented Nov 28, 2024

Description

Improves how we load various DLLs by preferring system32 DLLs, removing global SetDllDirectory changes, enabling safe path search mode and reducing risk of loading potentially wrong DLLs from the current directory.

Motivation and Context

Annoyed by various bug reports that suggest OBS is vulnerable to DLL planting attacks. While this PR doesn't aim to solve that issue directly, it implements best practices to help reduce the risk. We still rely on the current directory for relative path generation which isn't ideal, but that's for another PR.

How Has This Been Tested?

Tested loading OBS with default set of plugins. More testing with 3rd party plugins with complex dependency chains might be needed, though LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR should match the old behavior.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

This helps mitigate "DLL planting" attacks by removing the current
directory from most DLL loading calls.
Using the full path and appropriate LoadLibraryEx flags is safer as it
gives us more control over the search paths.
@WizardCM WizardCM added Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI Windows Affects Windows labels Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI Windows Affects Windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants