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

Add a security warning to Code.eval_string #5819

Merged
merged 2 commits into from
Feb 27, 2017
Merged

Add a security warning to Code.eval_string #5819

merged 2 commits into from
Feb 27, 2017

Conversation

ivan
Copy link
Contributor

@ivan ivan commented Feb 26, 2017

This PR adds a warning to the Code.eval_string docs about the security consequences of evaling untrusted strings.

You might think that this is obvious or unnecessary, but I've been finding serious security bugs caused by running eval_string on untrusted input: tonini/alchemist-server#14; msaraiva/atom-elixir#67

@@ -104,6 +104,10 @@ defmodule Code do
The `binding` argument is a keyword list of variable bindings.
The `opts` argument is a keyword list of environment options.

**Warning**: `string` is assumed to be fully trusted. If you receive strings
(for example, over the network), passing them into this function can execute
arbitrary code and compromise your machine.
Copy link
Member

@whatyouhide whatyouhide Feb 26, 2017

Choose a reason for hiding this comment

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

I am very 👍 to this warning. I would phrase it differently though:

string can be any Elixir code and will be executed in the system: this means that such code could compromise the machine (for example by executing system commands). Don't use eval_string/3 with untrusted input (such as strings coming from the network).

Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like your reworded version. But I don't really know what "executed in the system" specifically refers to - maybe "executed without any kind of sandbox" or "executed with full privileges"? Also, eval_string instead of eval_string/3 because it applies to /1 and /2 as well?

Copy link
Member

Choose a reason for hiding this comment

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

maybe "executed without any kind of sandbox" or "executed with full privileges"

What about "executed with the same privileges of the Erlang VM"?

Also, eval_string instead of eval_string/3 because it applies to /1 and /2 as well?

We always refer to the highest arity when a function is present in different arities because of default arguments. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated PR

@josevalim josevalim merged commit f1daca5 into elixir-lang:master Feb 27, 2017
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants