46

I've seen this multiple times in multiple places, but never have found a satisfying explanation as to why this should be the case.

So, hopefully, one will be presented here. Why should we (at least, generally) not use exec() and eval()?

EDIT: I see that people are assuming that this question pertains to web servers – it doesn't. I can see why an unsanitized string being passed to exec could be bad. Is it bad in non-web-applications?

Timur Shtatland
  • 12,024
  • 2
  • 30
  • 47
Isaac
  • 15,783
  • 9
  • 53
  • 76
  • 1
    Can you provide some quotes from "multiple places" to clarify what remarks you're basing this on. It's important to know what you read that lead to to conclude they should be avoided. It's not a simple issue, and you need to provide your source of information to clarify your question. – S.Lott Dec 19 '09 at 17:05
  • Generally, from what I recall (as I haven't bookmarked these places over the years), the warnings take the form of "of course, we could use `eval()` to solve this issue, but that'd be naughty." – Isaac Dec 19 '09 at 17:08
  • I suppose the most recent example was using `exec()` as a means of control flow within the program. – Isaac Dec 19 '09 at 17:09
  • 1
    Similar: http://stackoverflow.com/questions/637421/is-eval-supposed-to-be-nasty/637443#637443 – recursive Dec 19 '09 at 18:15
  • "Just don't use `eval" type comments can be found all over the place and I come across them on the daily. Here's one: https://stackoverflow.com/questions/8294618/define-a-lambda-expression-that-raises-an-exception#comment47551691_9547687 – JDG Nov 21 '20 at 02:12

11 Answers11

32

There are often clearer, more direct ways to get the same effect. If you build a complex string and pass it to exec, the code is difficult to follow, and difficult to test.

Example: I wrote code that read in string keys and values and set corresponding fields in an object. It looked like this:

for key, val in values:
    fieldName = valueToFieldName[key]
    fieldType = fieldNameToType[fieldName]
    if fieldType is int:
        s = 'object.%s = int(%s)' % (fieldName, fieldType) 
    #Many clauses like this...

exec(s)

That code isn't too terrible for simple cases, but as new types cropped up it got more and more complex. When there were bugs they always triggered on the call to exec, so stack traces didn't help me find them. Eventually I switched to a slightly longer, less clever version that set each field explicitly.

The first rule of code clarity is that each line of your code should be easy to understand by looking only at the lines near it. This is why goto and global variables are discouraged. exec and eval make it easy to break this rule badly.

RossFabricant
  • 12,364
  • 3
  • 41
  • 50
  • 4
    Straw man argument almost, you should never even get so far as to **build** a string to pass to exec. Passing a literal is bad enough. Isn't even more horrible to use exec if you don't even need it? `setattr` would do the job better. However, you are not so experienced with Python; `fieldType is int`? and `(int)%s` ?? – u0b34a0f6ae Dec 19 '09 at 17:38
  • 19
    It is not a straw man. What's the point of passing a literal to exec? As for your other comments. I typed in some quick code to illustrate a point, hoping that it would get my point across, and it would be easy to see how such a case could become more complex. It is true that my Python skills are a little rusty, but that has little bearing on the point I was making. – RossFabricant Dec 19 '09 at 18:52
  • For reference, the more explicit way to write the `s =` line is `setattr(object, fieldName, int(fieldType))` – wjandrea Oct 15 '21 at 03:59
17

When you need exec and eval, yeah, you really do need them.

But, the majority of the in-the-wild usage of these functions (and the similar constructs in other scripting languages) is totally inappropriate and could be replaced with other simpler constructs that are faster, more secure and have fewer bugs.

You can, with proper escaping and filtering, use exec and eval safely. But the kind of coder who goes straight for exec/eval to solve a problem (because they don't understand the other facilities the language makes available) isn't the kind of coder that's going to be able to get that processing right; it's going to be someone who doesn't understand string processing and just blindly concatenates substrings, resulting in fragile insecure code.

It's the Lure Of Strings. Throwing string segments around looks easy and fools naïve coders into thinking they understand what they're doing. But experience shows the results are almost always wrong in some corner (or not-so-corner) case, often with potential security implications. This is why we say eval is evil. This is why we say regex-for-HTML is evil. This is why we push SQL parameterisation. Yes, you can get all these things right with manual string processing... but unless you already understand why we say those things, chances are you won't.

bobince
  • 528,062
  • 107
  • 651
  • 834
12

Security aside, eval and exec are often marked as undesirable because of the complexity they induce. When you see a eval call you often don't know what's really going on behind it, because it acts on data that's usually in a variable. This makes code harder to read.

Invoking the full power of the interpreter is a heavy weapon that should be only reserved for very tricky cases. In most cases, however, it's best avoided and simpler tools should be employed.

That said, like all generalizations, be wary of this one. In some cases, exec and eval can be valuable. But you must have a very good reason to use them. See this post for one acceptable use.

Community
  • 1
  • 1
Eli Bendersky
  • 263,248
  • 89
  • 350
  • 412
  • It looks as though an answer was selected that **didn't** use `exec()`... are there cases where `exec()` or `eval()` are necessary/not ill-advised? – Isaac Dec 19 '09 at 17:21
  • Answer seconded. Security aside, eval and exec look like maintenance nightmares. – Vladimir Gritsenko Dec 19 '09 at 17:31
  • @Isaac, the answer was selected because i was looking for ways to accomplish the task without exec. In the general case, it was a good use for exec, which I ended up using as the methods became more arbitrary – Eli Bendersky Dec 19 '09 at 18:49
12

eval() and exec() can promote lazy programming. More importantly it indicates the code being executed may not have been written at design time therefore not tested. In other words, how do you test dynamically generated code? Especially across browsers.

vvvvv
  • 25,404
  • 19
  • 49
  • 81
  • 3
    +1: The "Fundamental Expectation" is that you have all the source. With exec() and eval() you don't -- trivially -- have *all* the source. – S.Lott Dec 19 '09 at 17:32
  • And what if "Fundamental Expectation" is satisfied? – IGRACH Jun 15 '23 at 01:10
10

In contrast to what most answers are saying here, exec is actually part of the recipe for building super-complete decorators in Python, as you can duplicate everything about the decorated function exactly, producing the same signature for the purposes of documentation and such. It's key to the functionality of the widely used decorator module (http://pypi.python.org/pypi/decorator/). Other cases where exec/eval are essential is when constructing any kind of "interpreted Python" type of application, such as a Python-parsed template language (like Mako or Jinja).

So it's not like the presence of these functions are an immediate sign of an "insecure" application or library. Using them in the naive javascripty way to evaluate incoming JSON or something, yes that's very insecure. But as always, its all in the way you use it and these are very essential functions.

zzzeek
  • 72,307
  • 23
  • 193
  • 185
6

I have used eval() in the past (and still do from time-to-time) for massaging data during quick and dirty operations. It is part of the toolkit that can be used for getting a job done, but should NEVER be used for anything you plan to use in production such as any command-line tools or scripts, because of all the reasons mentioned in the other answers.

You cannot trust your users--ever--to do the right thing. In most cases they will, but you have to expect them to do all of the things you never thought of and find all of the bugs you never expected. This is precisely where eval() goes from being a tool to a liability.

A perfect example of this would be using Django, when constructing a QuerySet. The parameters passed to a query accepts keyword arguments, that look something like this:

results = Foo.objects.filter(whatever__contains='pizza')

If you're programmatically assigning arguments, you might think to do something like this:

results = eval("Foo.objects.filter(%s__%s=%s)" % (field, matcher, value))

But there is always a better way that doesn't use eval(), which is passing a dictionary by reference:

results = Foo.objects.filter( **{'%s__%s' % (field, matcher): value} ) 

By doing it this way, it's not only faster performance-wise, but also safer and more Pythonic.

Moral of the story?

Use of eval() is ok for small tasks, tests, and truly temporary things, but bad for permanent usage because there is almost certainly always a better way to do it!

jathanism
  • 33,067
  • 9
  • 68
  • 86
  • +1 for telling that use of `eval()` is totally okay in home-grown scripts. – akuhn Dec 19 '09 at 19:46
  • 1
    Well, until said home-grown scripts leave the nest and become widely used, as they are wont to do. – Adriano Varoli Piazza Dec 19 '09 at 23:54
  • If you've ever used the decorator module in production (as well as a lot of other libs), you've used exec in production. exec and eval are useful for many things besides data input by an end user. – zzzeek Dec 20 '09 at 00:25
  • Sure, but if you have to ask why `exec()` and `eval()` should be avoided, you probably shouldn't be using them on your own. Not actively. – jathanism Dec 21 '09 at 00:49
5

Allowing these function in a context where they might run user input is a security issue, and sanitizers that actually work are hard to write.

dmckee --- ex-moderator kitten
  • 98,632
  • 24
  • 142
  • 234
  • 1
    So it's only bad to use in the context of a web application? – Isaac Dec 19 '09 at 17:03
  • 2
    Are all you users at the command line trusted? Maybe you should be concerned about that as well... – dmckee --- ex-moderator kitten Dec 19 '09 at 17:04
  • 2
    No. They could be used as part of privilege escalation attack by someone who doesn't have root access on the machine but does have access to run the script. – Rushyo Dec 19 '09 at 17:04
  • Technically, any place where exec() and eval() run inputs that are directly given or influenced by user is insecure. As a knowledgeable user I could make the code do what it shouldn't be doing. Wherever this situation occurs, it should be avoided. –  Dec 19 '09 at 17:11
5

Same reason you shouldn't login as root: it's too easy to shoot yourself in the foot.

Azeem.Butt
  • 5,855
  • 1
  • 26
  • 22
4

Don't try to do the following on your computer:

s = "import shutil; shutil.rmtree('/nonexisting')"
eval(s)

Now assume somebody can control s from a web application, for example.

ddejohn
  • 8,775
  • 3
  • 17
  • 30
Chris
  • 1,133
  • 7
  • 14
  • 6
    Just an FYI, that doesn't work. You can only embed one expression in an eval statement. That embeds two statements.... In fact, this might need a warning because I just now realized I just tried those two lines on my computer. :-/ – Jason Baker Dec 19 '09 at 17:11
  • 2
    what about __import__("shutil").rmtree, that's a single expression – Jeffrey Aylesworth Dec 19 '09 at 17:29
  • 11
    `"__import__('shutil').rmtree('/')"` is the variant that "works" -- it's a single expression so `eval` will happily destroy your filesystem (if the program's running with sufficient privilege). – Alex Martelli Dec 19 '09 at 17:29
  • 7
    1+ for using unnecessary dangerous command - and to Jeffrey for debugging it. – J.J Mar 01 '15 at 19:26
4

Reason #1: One security flaw (ie. programming errors... and we can't claim those can be avoided) and you've just given the user access to the shell of the server.

Rushyo
  • 7,495
  • 4
  • 35
  • 42
3

Try this in the interactive interpreter and see what happens:

>>> import sys
>>> eval('{"name" : %s}' % ("sys.exit(1)"))

Of course, this is a corner case, but it can be tricky to prevent things like this.

Jason Baker
  • 192,085
  • 135
  • 376
  • 510
  • You can also use exit() without having any module imported. – badp Dec 19 '09 at 20:08
  • 2
    I don't recall where it comes from, but exit isn't a built-in function. Somewhere in the documentation, it says that you shouldn't rely on this function existing except at the interactive interpreter. Of course, that doesn't matter much if you're trying to break into a system. :-) – Jason Baker Dec 19 '09 at 21:08