A `cmd.bat` in current directory could trick xonsh to run arbitrary code on Windows about xonsh

[y0umu]

does this still happen in Python 3.12?

Sadly it DOES happen in Python 3.12:

(xonsh_dbg_py312) PS D:\xzc\lab\xonsh_debug\xonsh-0.14.3> Get-Content .\cmd.bat
echo "hopefully this code is not malicious"
(xonsh_dbg_py312) PS D:\xzc\lab\xonsh_debug\xonsh-0.14.3> python --version
Python 3.12.1
(xonsh_dbg_py312) PS D:\xzc\lab\xonsh_debug\xonsh-0.14.3> $Env:XONSH_DEBUG=1
(xonsh_dbg_py312) PS D:\xzc\lab\xonsh_debug\xonsh-0.14.3> xonsh
<xonsh-code>:1:0 - cls
<xonsh-code>:1:0 + ![cls]
Skipping setup. Because no `readline` implementation available.
            Please install a backend (`readline`, `prompt-toolkit`, etc) to use
            `xonsh` interactively.
            See https://github.com/xonsh/xonsh/issues/1170
hello!
xonsh_dbg_py312xzc@Lenovo-Win11-xzc D:\xzc\lab\xonsh_debug\xonsh-0.14.3
@ mklink /?
<stdin>:1:8 - mklink /?
<stdin>:1:8 + ![mklink /?]

D:\xzc\lab\xonsh_debug\xonsh-0.14.3>echo "hopefully this code is not malicious"
"hopefully this code is not malicious"
xonsh_dbg_py312xzc@Lenovo-Win11-xzc D:\xzc\lab\xonsh_debug\xonsh-0.14.3
@ xonfig
<xonsh-code>:1:0 - xonfig
<xonsh-code>:1:0 + ![xonfig]
+------------------+-----------------------+
| xonsh            | 0.14.3                |
| Python           | 3.12.1                |
| PLY              | 3.11                  |
| have readline    | False                 |
| prompt toolkit   | None                  |
| shell type       | readline              |
| history backend  | json                  |
| pygments         | None                  |
| on posix         | False                 |
| on linux         | False                 |
| on darwin        | False                 |
| on windows       | True                  |
| on cygwin        | False                 |
| on msys2         | False                 |
| is superuser     | False                 |
| default encoding | utf-8                 |
| xonsh encoding   | utf-8                 |
| encoding errors  | surrogateescape       |
| xontrib          | []                    |
| RC file 1        | C:\Users\xzc/.xonshrc |
+------------------+-----------------------+

I haven't checked if xonsh uses subprocess.run with shell=True

I searched for shell= and .run in the source but didn't find that. I don't know if I missed some class defaults though.

[JamesParrott]

With this line changed:

xonsh/xonsh/aliases.py

Line 897 in e3a6dcf

default_aliases[alias] = ["cmd", "/c", alias]

        for alias in windows_cmd_aliases:
            # default_aliases[alias] = ["cmd", "/c", alias]
            default_aliases[alias] = [os.getenv("COMSPEC"), "/c", alias]

this is the new, hopefully safer, behaviour:

(xonsh)  ~\Coding\delete @ mklink /?
Creates a symbolic link.

MKLINK [[/D] | [/H] | [/J]] Link Target

        /D      Creates a directory symbolic link.  Default is a file
                symbolic link.
        /H      Creates a hard link instead of a symbolic link.
        /J      Creates a Directory Junction.
        Link    Specifies the new symbolic link name.
        Target  Specifies the path (relative or absolute) that the new link
                refers to.
(xonsh)  ~\Coding\delete @ dir /A
 Volume in drive C is Windows-SSD
 Volume Serial Number is 

 Directory of ~\Coding\delete

24/12/2023  16:08    <DIR>          .
12/12/2023  12:03    <DIR>          ..
24/12/2023  13:43                43 cmd.bat
24/12/2023  16:06                32 legit_user_file.bat
12/11/2023  20:57    <DIR>          Project1
               2 File(s)             75 bytes
               3 Dir(s)  lots bytes free

[y0umu]

BTW my cmd.bat had also (unintentionally) tricked Visual Studio Code's python debugger to run something else while I was looking into this issue, as it uses cmd /c too.

[JamesParrott]

I haven't checked if xonsh uses subprocess.run with shell=True, but does this still happen in Python 3.12?

It reminds me of this bugfix:

Changed in version 3.12: Changed Windows shell search order for shell=True. The current directory and %PATH% are replaced with %COMSPEC% and %SystemRoot%\System32\cmd.exe. As a result, dropping a malicious program named cmd.exe into a current directory no longer works.

https://docs.python.org/3/library/subprocess.html

[JamesParrott]

Hiya,

I've gotten to the bottom of it.

Basically the problem is in Xonsh's prefilled default aliases on Windows, which include mklink but also dir:

A fix could be as simple as replacing the string literal "cmd" with os.getenv('COMSPEC')

    if ON_WINDOWS:
        # Borrow builtin commands from cmd.exe.
        windows_cmd_aliases = {
            "cls",
            "copy",
            "del",
            "dir",
            "echo",
            "erase",
            "md",
            "mkdir",
            "mklink",
            "move",
            "rd",
            "ren",
            "rename",
            "rmdir",
            "time",
            "type",
            "vol",
        }
        for alias in windows_cmd_aliases:
            default_aliases[alias] = ["cmd", "/c", alias]

xonsh/xonsh/aliases.py

Line 875 in e3a6dcf

if ON_WINDOWS:

Proof:

Run a command that isn't in the aliases dict, e.g. w32tm /tz (try to only pick examples that are harmless https://ss64.com/nt/), or make another batch file (and call it from xonsh) in the same directory as the cmd.bat imposter. The attacker's cmd.bat is not called.

(xonsh) ~\Coding\delete @ mklink /?

~\Coding\delete>echo "hopefully this code is not malicious"
"hopefully this code is not malicious"
(xonsh) ~\Coding\delete @ dir /A

~\Coding\delete>echo "hopefully this code is not malicious"
"hopefully this code is not malicious"
(xonsh) ~\Coding\delete @ w32tm /tz
Time zone: Current:TIME_ZONE_ID_STANDARD Bias: 0min (UTC=LocalTime+Bias)
  [Standard Name:"GMT Standard Time" Bias:0min Date:(M:10 D:5 DoW:0)]
  [Daylight Name:"GMT Daylight Time" Bias:-60min Date:(M:3 D:5 DoW:0)]
(xonsh) ~\Coding\delete @ legit_user_file

~\Coding\delete>echo A user's custom batch file.
A user's custom batch file.
(xonsh) ~\Coding\delete @

FYI, Xonsh uses a bunch of suprocess.capture_output s (and the other olf one) but for its main external calls it runs subprocess.POpen, via a custom class and the entire AST and parser. It took me a long time to figure that out, and will also take a long time to explain the details.

[y0umu]

With this line changed:

xonsh/xonsh/aliases.py

Line 897 in e3a6dcf

default_aliases[alias] = ["cmd", "/c", alias]

        for alias in windows_cmd_aliases:
            # default_aliases[alias] = ["cmd", "/c", alias]
            default_aliases[alias] = [os.getenv("COMSPEC"), "/c", alias]

this is the new, hopefully safer, behaviour:

(xonsh)  ~\Coding\delete @ mklink /?
Creates a symbolic link.

MKLINK [[/D] | [/H] | [/J]] Link Target

        /D      Creates a directory symbolic link.  Default is a file
                symbolic link.
        /H      Creates a hard link instead of a symbolic link.
        /J      Creates a Directory Junction.
        Link    Specifies the new symbolic link name.
        Target  Specifies the path (relative or absolute) that the new link
                refers to.
(xonsh)  ~\Coding\delete @ dir /A
 Volume in drive C is Windows-SSD
 Volume Serial Number is 

 Directory of ~\Coding\delete

24/12/2023  16:08    <DIR>          .
12/12/2023  12:03    <DIR>          ..
24/12/2023  13:43                43 cmd.bat
24/12/2023  16:06                32 legit_user_file.bat
12/11/2023  20:57    <DIR>          Project1
               2 File(s)             75 bytes
               3 Dir(s)  lots bytes free

This is good enough to handle the cmd hijacking issue. But maybe xonsh should go one step further. For example completely ignore executables in the current directory, like what PowerShell or most *nix shells do. If the user should execute something in the working directory, they must prepend it with .\ (PowerShell) or ./ (*nix shells).

[JamesParrott]

It is reasonable to expect Windows cmd users to check the directory first for executables and batch files of that name, before running that name as a command. That's what they should be doing already.

It is not reasonable to expect Windows cmd users of Xonsh to check every directory for a file called cmd.bat, when the very act of looking for a malicious cmd.bat using dir /A in Xonsh, would execute it.

If running on top of cmd, Xonsh isn't now any less secure than cmd (or won't be, once my PR is merged, or the issue is fixed some other way).

In the code base, there was a deliberate decision to support windows / cmd users running scripts without specifying a file extension or .\ .

xonsh/xonsh/commands_cache.py

Line 260 in e3a6dcf

# Windows users expect to be able to execute files in the same

        if ON_WINDOWS:
            # Windows users expect to be able to execute files in the same
            # directory without `./`
            local_bin = next((fn for fn in possibilities if os.path.isfile(fn)), None)

A strong case needs to be made to push out such a wide ranging breaking change.