Repository navigation
Never resolve a bare name to a batch file on the overlay PATH [patch] - #140
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
Since #137, a bare name with no extension on Windows was tried with every PATHEXT extension, so `npm` resolved to `npm.cmd`. CreateProcess starts a batch file through cmd.exe, which re-parses the command line, and the argument-vector escaping follows CommandLineToArgvW rather than cmd.exe's rules, so `&`, `|` or `%` in an argument ran as commands (the BatBadBut class, CVE-2024-24576). A name with no extension is now tried with `.exe` only, which is all CreateProcess adds, restoring the pre-#137 set of files a bare name can start. A batch file named with its extension is still the caller's choice. The README says so and warns against passing it untrusted arguments. The PATHEXT test is replaced by one that a bare name resolves to tool.exe and one that it never resolves to tool.cmd, tool.bat or tool.com while an explicit tool.cmd still does. Fixes #138 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5DyciGEV4wZvik3pQ
|
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
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.
Fixes #138
What changed
Since #137,
ResolveOnSearchPathtried a Windows name with no extension against everyPATHEXTextension, soExecuteAsync("npm", ...)withPATHin the overlay resolved tonpm.cmd. CreateProcess starts a batch file throughcmd.exe, which re-parses the whole command line. The argument escaping followsCommandLineToArgvWrather than cmd.exe's rules, so&,|or%in an argument ran as commands. This is the BatBadBut class (CVE-2024-24576).This takes option 1 from the issue and triage comment: a name with no extension is now tried with
.exeonly. That is the one extension CreateProcess adds, so a bare name can start the same set of files it could before #137. A batch file named with its extension (npm.cmd) is still resolved, because naming it is the caller's explicit choice. The README now says this and warns against passing untrusted arguments to a batch file.ResolveOnSearchPathno longer readsPATHEXT, so its unused environment parameter is gone. The method is internal.Tests
ResolveOnSearchPathShouldApplyPathExtOnWindowswith:ResolveOnSearchPathShouldAddExeToANameWithNoExtensionOnWindows: a bare name findstool.exe.ResolveOnSearchPathShouldNeverResolveABareNameToABatchFileOnWindows: withtool.cmd,tool.batandtool.comon the path, a baretoolresolves to nothing, and an explicittool.cmdstill resolves..cmdpick back in temporarily and the new test failed (Expected value to be null. actual: ".../tool-....cmd"). With the fix in place it passes.The resolution logic is tested through
isWindows: trueon Linux. I did not run the end-to-end cmd.exe repro on Windows, and the issue says its own repro was also worked out from the code.Generated by Claude Code