-
Notifications
You must be signed in to change notification settings - Fork 212
Security: Remove unsafe PowerShell fallback in WSL #866
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
base: dev
Are you sure you want to change the base?
Conversation
The existing fallback mechanism uses subprocess.call with an unescaped formatted string to invoke Start-Process. This introduces a command injection vulnerability where a malicious auth_uri can execute arbitrary PowerShell commands. This patch removes the vulnerable fallback path entirely. The webbrowser standard library is sufficient for handling URL opening across platforms.
rayluo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want to hear second opinion from @jiasli
|
Hi @rayluo, Thank you for the feedback. I completely agree that preserving WSL support is essential to avoid a breaking change. I have updated the PR with a much more secure implementation:
All 52 tests passed successfully. Looking forward to your and @jiasli's review! |
|
@microsoft-github-policy-service agree |
|
My apologies, @rayluo! That was definitely not intentional. I was using some local environment scripts to help manage and format the changes, and it seems some of those internal tool notes and metadata accidentally leaked into the file during the push. I've just pushed a clean commit to restore |
Problem
The previous implementation constructed a PowerShell command using string formatting with the
auth_uri. This pattern is susceptible to argument injection, potentially allowing arbitrary code execution if theauth_uriis controlled by an attacker.Solution
The vulnerable code block has been removed. The library now relies solely on Python's standard
webbrowsermodule, which handles URL opening safely and is the preferred method for cross-platform compatibility.