fix: WMI DBNull crash in drive scan and uninstaller prefix bypass#817
Merged
Conversation
Round 3 (P1 crash/security safety): - FixedDriveService read MediaType/BusType with Convert.ToUInt32(value ?? 0u). WMI returns DBNull.Value (not null) for absent properties, so the ?? never fired and Convert.ToUInt32(DBNull.Value) threw, aborting the whole drive scan on some hardware. Reads now use a ToUInt32Safe helper (null and DBNull -> 0). - UninstallerService.IsUnderTrustedDirectory used a bare StartsWith, letting 'C:\Program Files Evil\x.exe' pass the 'C:\Program Files' check. It now compares on a normalized directory boundary so only real sub-paths qualify. Both methods made internal for direct unit testing (InternalsVisibleTo already present). Tests cover DBNull/null/convertible/unconvertible inputs and the sibling-prefix bypass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Round 3 of the audit — P1 crash/security safety (the two highest-impact items in this round).
Drive enumeration crashed on missing WMI properties
FixedDriveServicereadMediaType/BusTypeviaConvert.ToUInt32(value ?? 0u). WMI returnsDBNull.Value(not null) for absent properties, so the??never fired andConvert.ToUInt32(DBNull.Value)threw — aborting the entire drive scan on hardware that doesn't report those fields. Reads now go through aToUInt32Safehelper that maps both null andDBNullto 0.Uninstaller trusted-directory check accepted sibling folders
IsUnderTrustedDirectoryused a bareStartsWith, soC:\Program Files Evil\malware.exepassed theC:\Program Filescheck. It now compares on a normalized directory boundary (trailing separator +GetFullPath), so only genuine sub-paths of a trusted directory qualify.Tests
Both methods made
internal(InternalsVisibleTo already present) for direct unit testing:ToUInt32Safe: DBNull → 0, null → 0, convertible values, unconvertible string → 0.IsUnderTrustedDirectory: path inside Program Files trusted;Program Files Evilsibling rejected; unrelated temp path rejected.Regression
Full-solution build clean (main + Tests + IntegrationTests + UITests). Version 1.20.3.
Remaining Round-3 items (PingMonitor CTS, LogsVM UI-thread COM, StartupVM cross-thread, Dashboard NVIDIA-only GPU) will follow in a separate PR.