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

Experimental feature PSNativeCommandArgumentPassing doesn't support arguments with embedded double quotes when calling batch files #15250

Open
mklement0 opened this issue Apr 16, 2021 · 5 comments
Assignees
Labels
Issue-Bug Issue has been identified as a bug in the product WG-Engine core PowerShell engine, interpreter, and runtime

Comments

@mklement0
Copy link
Contributor

mklement0 commented Apr 16, 2021

Similar to #15239, this issue would be resolved by the proposal in #15143. See also: #15261 and #15276

Note that high-profile CLIs such as az and npm (Node.js's package manager), for script-based utilities that come with packages, use batch files as their entry points.

Steps to reproduce

Run the following on Windows:

# Switch to a temporary directory.
Push-Location -ea Stop ($tmpDir = (New-Item -Type Directory -Force (Join-Path Temp:/ $PID)).FullName)

# Create a batch file that strips enclosing double quotes from the first argument and deduplicates the 
# embedded ones.
@'
@echo off & setlocal
set Arg1=%~1
echo [%Arg1:""="%] [%2]
'@ > foo.cmd

./foo.cmd 'Andre "The Hawk" Dawson' | Should -Be '[Andre "The Hawk" Dawson] []'

# Clean up.
Pop-Location; Remove-Item $tmpDir -Recurse

Expected behavior

The test should succeed.

Actual behavior

The test fails, because the argument with embedded " isn't passed batch-file-appropriately - batch files (cmd.exe) do not recognize \" as an escaped " and require "" instead:

InvalidResult: Expected strings to be the same, but they were different. 
Expected length: 28 Actual length:   30 Strings differ at index 7. 
Expected: '[Andre "The Hawk" Dawson] []' 
But was:  '[Andre \"The] [Hawk\" Dawson"]'

Note the literally retained \ and the unexpected breakup into two arguments.

For this invocation to succeed it would have to pass verbatim Andre "The Hawk" Dawson as "Andre ""The Hawk"" Dawson" - i.e. escaping the embedded " as "" rather than \" - on the command line used behind the scenes.

Environment data

PowerShell Core 7.2.0-preview.5 on Windows
@mklement0 mklement0 added the Needs-Triage The issue is new and needs to be triaged by a work group. label Apr 16, 2021
@mklement0 mklement0 changed the title Experimental feature PSNativeCommandArgumentPassing doesn't support with embedded double quotes when calling batch files Experimental feature PSNativeCommandArgumentPassing doesn't support arguments with embedded double quotes when calling batch files Apr 16, 2021
@iSazonov iSazonov added the WG-Engine core PowerShell engine, interpreter, and runtime label Apr 16, 2021
@SteveL-MSFT
Copy link
Member

The proposed fix would be to special case cmd.exe and files ending with .bat or .cmd to pass enclosed double quotes as ""?

@mklement0
Copy link
Contributor Author

mklement0 commented Apr 18, 2021

Indeed, @SteveL-MSFT.

It is part of what I've been proposing for months, repeatedly, in conversations you were a part of: here and here and here, which finally evolved into a - hopefully comprehensive and final - proposal in #15143, which summarizes all accommodations that I think are vital to make on Windows, the need to escape " as "" for batch files being just one of them.

Excuse me for shouting, but it's out of desperation over these issues seemingly never being paid proper attention to; this isn't a personal concern; rather, I think these issues are of vital importance for all PowerShell users, and we should finally get them right.

Therefore, I ask you to please take the time to fully understand and discuss:


Moved to #15143 (comment)

@mklement0
Copy link
Contributor Author

@SteveL-MSFT, I've moved the gist of my previous comment to #15143 (comment), to have all the relevant information in one place.

Note that a new bug has since joined the ranks: #15276

@daxian-dbw daxian-dbw added Issue-Bug Issue has been identified as a bug in the product and removed Needs-Triage The issue is new and needs to be triaged by a work group. labels Jun 9, 2021
@seanbudd
Copy link

Any updates on this bug?

seanbudd added a commit to nvaccess/addon-datastore that referenced this issue Jun 26, 2023
summary of issue
#992 failed without feedback to the user

This is because the download failed, due to an ampersand in the query parameters, causing the end of the URL to be interpreted as a separate command.
This is a minor security risk, as it could cause remote code injection.

Additionally, users should be alerted as to why their download has failed, instead of failing silently.

Dev process
PowerShell has a bug with stripping quotes when passing to batch scripts (PowerShell/PowerShell#15250).
Additionally, no validation error is written to file if the downloaded file was not a valid zip file.
Refer to nvaccess/addon-datastore-validation#31 where these issues are fixed.

testing
See example nvaccess#11 where a comment was successfully posted and urls were successfully quoted via this action run
seanbudd added a commit to nvaccess/addon-datastore-validation that referenced this issue Jun 26, 2023
…valid (#31)

See also: nvaccess/addon-datastore#1008

PowerShell has a bug with stripping quotes when passing to batch scripts (PowerShell/PowerShell#15250).
Additionally, no validation error was written to file if the downloaded file was not a valid zip file.

testing
See example nvaccess/addon-datastore-staging#11 where a comment was successfully posted and urls were successfully quoted via this action run
@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution-No Activity Issue has had no activity for 6 months or more label Dec 23, 2023
@seanbudd
Copy link

Bump to keep open

@microsoft-github-policy-service microsoft-github-policy-service bot removed the Resolution-No Activity Issue has had no activity for 6 months or more label Dec 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue has been identified as a bug in the product WG-Engine core PowerShell engine, interpreter, and runtime
Projects
None yet
Development

No branches or pull requests

6 participants