Categories
eval python

Why is using ‘eval’ a bad practice?

172

I use the following class to easily store data of my songs.

class Song:
    """The class to store the details of each song"""
    attsToStore=('Name', 'Artist', 'Album', 'Genre', 'Location')
    def __init__(self):
        for att in self.attsToStore:
            exec 'self.%s=None'%(att.lower()) in locals()
    def setDetail(self, key, val):
        if key in self.attsToStore:
            exec 'self.%s=val'%(key.lower()) in locals()

I feel that this is just much more extensible than writing out an if/else block. However, I have heard that eval is unsafe. Is it? What is the risk? How can I solve the underlying problem in my class (setting attributes of self dynamically) without incurring that risk?

4

  • 48

    how did you learn about exec/eval and still didn’t know setattr?

    Dec 2, 2009 at 14:59

  • 4

    I believe it was from an article comparing python and lisp than I learned about eval.

    – Nikwin

    Dec 2, 2009 at 16:05

  • This should have been considered as two separate questions in the first place – explaining the risk of eval, and showing how to replace this specific usage. However, this question is much too important as a canonical duplicate to do much about that.

    May 30 at 0:36

  • See also: Using setattr() in python

    May 30 at 0:37


228

Yes, using eval is a bad practice. Just to name a few reasons:

  1. There is almost always a better way to do it
  2. Very dangerous and insecure
  3. Makes debugging difficult
  4. Slow

In your case you can use setattr instead:

class Song:
    """The class to store the details of each song"""
    attsToStore=('Name', 'Artist', 'Album', 'Genre', 'Location')
    def __init__(self):
        for att in self.attsToStore:
            setattr(self, att.lower(), None)
    def setDetail(self, key, val):
        if key in self.attsToStore:
            setattr(self, key.lower(), val)

There are some cases where you have to use eval or exec. But they are rare. Using eval in your case is a bad practice for sure. I’m emphasizing on bad practice because eval and exec are frequently used in the wrong place.

Replying to the comments:

It looks like some disagree that eval is ‘very dangerous and insecure’ in the OP case. That might be true for this specific case but not in general. The question was general and the reasons I listed are true for the general case as well.

16

  • 27

    -1: “Very dangerous and insecure” is false. The other three are outstandingly clear. Please reorder them so that 2 and 4 are the first two. It’s only insecure if you are surrounded by evil sociopaths who are looking for ways to subvert your application.

    – S.Lott

    Dec 2, 2009 at 15:41

  • 66

    @S.Lott, Insecurity is a very important reason to avoid eval/exec in general. Many applications like websites should take extra care. Take the OP example in a website that expects users to enter the song name. It is bound to be exploited sooner or later. Even an innocent input like: Let’s have fun. will cause a syntax error and expose the vulnerability.

    Dec 2, 2009 at 16:20

  • 24

    @Nadia Alramli: User input and eval have nothing to do with each other. An application that’s fundamentally mis-designed is fundamentally mis-designed. eval is no more the root cause of bad design than division by zero or attempting to import a module which is known not to exist. eval isn’t insecure. Applications are insecure.

    – S.Lott

    Dec 2, 2009 at 17:38

  • 19

    @jeffjose: Actually, it is fundamentally bad/evil because it’s treating unparamaterized data as code (this is why XSS, SQL injection, and stack smashes exist). @S.Lott: “It’s only insecure if you are surrounded by evil sociopaths who are looking for ways to subvert your application.” Cool, so say you make a program calc, and to add numbers it executes print(eval("{} + {}".format(n1, n2))) and exits. Now you distribute this program with some OS. Then someone makes a bash script that takes some numbers from a stock site and adds them using calc. boom?

    Jul 4, 2010 at 17:23


  • 69

    I’m not sure why Nadia’s assertion is so contentious. It seems simple to me: eval is a vector for code injection, and is dangerous in a way that most other Python functions are not. That doesn’t mean you shouldn’t use it at all, but I think you should use it judiciously.

    – Owen S.

    Aug 19, 2011 at 16:52

45

Using eval is weak, not a clearly bad practice.

  1. It violates the “Fundamental Principle of Software”. Your source is not the sum total of what’s executable. In addition to your source, there are the arguments to eval, which must be clearly understood. For this reason, it’s the tool of last resort.

  2. It’s usually a sign of thoughtless design. There’s rarely a good reason for dynamic source code, built on-the-fly. Almost anything can be done with delegation and other OO design techniques.

  3. It leads to relatively slow on-the-fly compilation of small pieces of code. An overhead which can be avoided by using better design patterns.

As a footnote, in the hands of deranged sociopaths, it may not work out well. However, when confronted with deranged sociopathic users or administrators, it’s best to not give them interpreted Python in the first place. In the hands of the truly evil, Python can a liability; eval doesn’t increase the risk at all.

11

  • 7

    @Owen S. The point is this. Folks will tell you that eval is some kind of “security vulnerability”. As if Python — itself — was not just a bunch of interpreted source that anyone could modify. When confronted with the “eval is a security hole”, you can only assume that it’s a security hole in the hands of sociopaths. Ordinary programmers merely modify the existing Python source and cause their problems directly. Not indirectly through eval magic.

    – S.Lott

    Aug 19, 2011 at 21:34

  • 16

    Well, I can tell you exactly why I would say eval is a security vulnerability, and it has to do with the trustworthiness of the string it’s given as input. If that string comes, in whole or in part, from the outside world, there’s a possibility of a scripting attack on your program if you’re not careful. But that’s thge derangement of an outside attacker, not of the user or administrator.

    – Owen S.

    Jan 6, 2012 at 18:42


  • 7

    @OwenS.: “If that string comes, in whole or in part, from the outside world” Often false. This isn’t a “careful” thing. It’s black and white. If the text comes from a user, it can never be trusted. Care isn’t really part of it, it’s absolutely untrustable. Otherwise, the text comes from a developer, installer or admin, and can be trusted.

    – S.Lott

    Jan 6, 2012 at 19:08

  • 9

    @OwenS.: There’s no possible escaping for a string of untrusted Python code that would make it trustable. I agree with most of what you’re saying except for the “careful” part. It’s a very crisp distinction. Code from the outside world is untrustable. AFAIK, no amount of escaping or filtering can clean it up. If you have some kind of escaping function that would make code acceptable, please share. I didn’t think such a thing was possible. For example while True: pass would be hard to clean up with some kind of escaping.

    – S.Lott

    Jan 6, 2012 at 22:54


  • 2

    @OwenS.: “intended as a string, not arbitrary code”. That’s unrelated. That’s just a string value, which you would never pass through eval(), since it’s a string. Code from the “outside world” cannot be sanitized. Strings from the outside world are just strings. I’m unclear on what you’re talking about. Perhaps you should provide a more complete blog post and link to it here.

    – S.Lott

    Jan 9, 2012 at 0:01

24

Yes, it is:

Hack using Python:

>>> eval(input())
"__import__('os').listdir('.')"
...........
...........   #dir listing
...........

The below code will list all tasks running on a Windows machine.

>>> eval(input())
"__import__('subprocess').Popen(['tasklist'],stdout=__import__('subprocess').PIPE).communicate()[0]"

In Linux:

>>> eval(input())
"__import__('subprocess').Popen(['ps', 'aux'],stdout=__import__('subprocess').PIPE).communicate()[0]"

2

  • Why is that bad/dangerous? Can’t I just execute the same Python code anyway without eval?

    – mkrieger1

    Apr 17 at 18:45


  • It is dangerous because it allows for text that is not the intentionally written source code of the program to be used as if it were source code. This means that you cannot feed your program with data that came from another source (such as an Internet download, a web submission form, a keyboard at a public kiosk…) without allowing arbitrary code execution on the computer where the program runs. This is fundamentally the same problem as SQL injection, except worse because it has access to an entire computer, not just a database.

    May 30 at 0:31