Trivial pollution of workbench.action.terminal.runRecentCommand list

[davidje13]

Does this issue occur when all extensions are disabled?: Yes

• VS Code Version: 1.70.1
• OS Version: macOS 12.5

This does not appear to meet the definition of a security vulnerability as defined in your security policy, but is concerning so I am raising it as a standard bug report instead.

---

Steps to Reproduce:

1. Run this innocent-looking curl request in the vscode terminal:

   curl https://davidje13.github.io/vscode-command-test/info.txt

   (you can see the file being downloaded and the plain-text script used to generate it here: https://github.com/davidje13/vscode-command-test)

   It will output: "Hello here is some legitimate content"

2. Run the command workbench.action.terminal.runRecentCommand (e.g. by using Cmd+R / Ctrl+R if the suggestion in the documentation has been followed)

3. Notice that make (and possibly npm install, but adding multiple commands at once seems unreliable) have appeared in the list. Notice that running one of these will execute the expected command, followed by the (nonexistent) badcommand executable (which in practice could obviously be anything)

   

This example uses curling a URL, but it applies to any situation where a malicious actor can manipulate output displayed in the terminal as the result of any command (e.g. npm package installation info even if scripts are disabled, or server log output when running a local server via this terminal).

Fortunately this list appears to only be used (so far) for the command history feature, which is not enabled by default.

---

There are 2 issues here:

1. it is possible to silently register commands in this list — even from locations which only have the ability to output text — by using the VSCode escape sequence \033]633;E;%s\007 (the example above adds some other sequences around it to improve the reliability when registering multiple commands at once)
2. It is possible to hide malicious parts of a command by adding arbitrary amounts of whitespace (in the example above, the "legitimate" part of the command is followed by many spaces to ensure the "malicious" part is hidden from view, and the only indication to a user that something is amiss before they run it is the "…")

For the first issue, adding a per-session or per-installation random "password" to the escape sequence may be enough to prevent most attacks (as accessing this "password" would likely require script level access anyway)

For the second issue, collapsing multiple spaces in the display might help, but there are likely other ways to obfuscate this sort of thing.

[Tyriar]

Thanks for the report! We had a chat about this and feel solving the second problem mostly solves the first problem, while at the same time not restricting or making the protocol we use overly complex. Current proposal:

• Collapse 3+ spaces into ⋯ which resembles how whitespace displays in the editor, this is similar to our \n -> ⏎ conversion
  • Ideally this would be a different color to differentiate, that's blocked on Support SGR ANSI escape sequences in quick pick labels #158192 though
• For long commands, maybe leverage the quick pick "detail" to show a second line and potentially adding a button to preview the command. We're still thinking through this

For the password/nonce approach we could go this route, but given the actions above I'm more concerned about programs misusing the protocol to fill the commands list with unwanted commands than I am about malicious commands. We can look into this at a later time if we find it's needed.

We could also just add a dialog for the user to confirm a long command but I'd really like to not take away from the usability of the feature. It's possible future iterations will use something closer to the auto-complete UI in the editor as well at which point we could put the full command in the detail.

[davidje13]

@Tyriar that avoids the super-trivial attack, but it's pretty easy to work around that protection:

echo "Hello here is some legitimate content" > info.txt
PAD="  \342\201\257"
MPAD="$PAD$PAD$PAD$PAD$PAD$PAD$PAD$PAD$PAD$PAD"
printf "\033]633;E;npm install$MPAD$MPAD$MPAD$MPAD$MPAD$MPAD$MPAD$MPAD$MPAD$MPAD & badcommand\007" >> info.txt

Definitely much more noticeable when it executes, but by then it's too late (and I'm sure more subtle ways can be found given some effort; this was just the first thing that came to mind)

[Tyriar]

@davidje13 thanks again 🙂. That's where the details to call out long commands and let you preview them comes in. We're also thinking about supporting multiple lines in the details part which could let us preview the entire command #153095

[Tyriar]

Clarifying status: this is a feature request that's blocked on #153095 landing