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

open_port/2 on Windows #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

@voltone
Copy link
Collaborator Author

voltone commented Apr 11, 2024

Not sure the comment to include the extension when running binaries is really necessary: we later say that "note that the {spawn_executable, FileName} form requires specifying the full path to the executable", so it looks like the extension would be required anyway, and a .bat/.cmd file with the same name wouldn't be accidentally run instead...

@@ -27,7 +27,7 @@ Sometimes it may be necessary to spawn an external executable to complete a task

The [os:cmd/1,2](https://erlang.org/doc/man/os.html#cmd-2) function in the Erlang/OTP standard library takes a single argument containing both the name of the executable and any command line arguments. An operating system shell is spawned to parse the command and locate the executable. This makes it particularly difficult to protect against command injection, requiring careful filtering and escaping of untrusted input prior to inclusion in the command.

It is safer to use [open_port/2](https://erlang.org/doc/man/erlang.html#open_port-2) with `{spawn_executable, FileName}`, which takes the command line arguments as a list, through the `{args, Args}` option. The arguments are passed to the executable as-is, without environment variable expansion or other processing, neutralizing injection attacks.
It is safer to use [open_port/2](https://erlang.org/doc/man/erlang.html#open_port-2) with `{spawn_executable, FileName}`, which takes the command line arguments as a list, through the `{args, Args}` option. The arguments are passed to the executable as-is, without environment variable expansion or other processing, neutralizing injection attacks. Note, however, that executing batch files (.bat, .cmd, ...) on Microsoft Windows may not be safe, even with `open_port/2`, as mentioned [here](https://erlangforums.com/t/user-controlled-arguments-to-open-port-2-with-spawn-spawn-executable-is-insecure-on-windows/3476). When executing binaries on Windows, explicitly specify the extension (e.g. .exe) to avoid accidental invocation of a batch file.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@voltone I'd prefer if we wrote the reasoning in our guidelines instead of linking outside.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@voltone Maybe something like this?

A vulnerability has been identified across multiple programming languages on Microsoft Windows where arguments passed to commands are not properly escaped. This can lead to unintended command execution, posing a security risk. As fixing this without breaking backwards compatibility is not possible, users are advised to refrain from passing untrusted inputs as arguments to open_port/2.

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

Successfully merging this pull request may close these issues.

2 participants