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

Handle '----' better and enforce having a space before it. #907

Closed
aeloros opened this issue Jun 3, 2021 · 3 comments
Closed

Handle '----' better and enforce having a space before it. #907

aeloros opened this issue Jun 3, 2021 · 3 comments
Assignees

Comments

@aeloros
Copy link
Contributor

aeloros commented Jun 3, 2021

It depends. If the command line is

----a:b"c\"d"e

is the expected parse

  1. contract = a, data = b"c\"d"e; or
  2. contract = a, data = bc"de
    CommandLineToArgvW will give you version 2.

Also, it seems that a space is not required before the first dash, so we will treat

contoso.exe hello----dolly.txt

as

  • contract = dolly.txt, data = empty

which could be surprising if somebody has a file named "hello----dolly.txt".

Originally posted by @oldnewthing in #823 (comment)

@aeloros
Copy link
Contributor Author

aeloros commented Jun 3, 2021

Make sure to write tests to validate command-line behaviors.

@oldnewthing
Copy link
Member

oldnewthing commented Jun 17, 2021

Even a space before the dash isn't good enough. The user might have a file called "hello ----dolly.txt" and launching it will pass the command line contoso.exe "hello ----dolly.txt" and since we don't track quotation marks, we think that the space before the dashes is a command line operation separator when in fact it's part of an enclosing file name.

This can be used as a command line injection attack: Create a file called "C:\x ----ms-protocol" with an alternate stream named "attack-you-here .txt". Launching that file will pass the command line "C:\x ----ms-protocol::attack-you-here .txt" which will get mis-parsed as a ms-protocol launch with URL "attack-you-here" (which can be replaced with any desired attack string).

@aeloros
Copy link
Contributor Author

aeloros commented Aug 5, 2021

Address these items as part of this issue.

#823 (comment)
#823 (comment)

@aeloros aeloros closed this as completed Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants