Why is using 'eval' a bad practice?

Question

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?

Flong

how did you learn about exec/eval and still didn't know setattr? – Christyna

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

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. – Clavius

See also: Using setattr() in python – Clavius

Answer 1

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.

Lenticular

I like this better than __dic__ since it's more explicit on what you intend to do. – Maurits

In this case, eval is a bad practice. But eval is not a universal bad practice. There are cases where using eval/exec/execfile is great. – External

-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. – Numbersnumbfish

@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. – Lenticular

If eval is evil, why do it exists, in the first place? – Straw

@Selinap, eval has practical use cases but they are rare and special. In the OP case it is very very clear eval is a bad and insecure choice. If something exists that doesn't mean we should blindly use it. – Lenticular

I agree with S.Lott: there is no user data in executed statement, so there is no reason to consider it insecure. – Rockafellow

@Denis, you are right the OP case doesn't have user data. But still the question was general: "Is Using eval In Python A Bad Practice?" – Lenticular

@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. – Numbersnumbfish

S.Lott's point is really valid. eval fundamentally isnt bad/evil. Its the design which is almost always to blame. If your design doesnt guarantee you against evil user-input, eval might not be what you should be worried about. – Patrilocal

@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? – Spinoza

@S.Lott: It's probably better to say that eval is sane if and only if the method using eval doesn't execute insane code as a side effect... Say I have a JSON decoder in JavaScript... I want to pass anything to it, I don't want to waste my time writing sanitization code every time I call it (which would be the case if the function just returned eval(input). – Spinoza

Look, everyone using Python isn't writing a web script or application where security is a major concern. So, while I agree that there's often a better way, say using setattr(), and that one should make the effort to find them for a better design, I definitely don't agree that eval/exec are inherently 'very dangerous and insecure'. There's good reasons they're available and having the abilities they provide is one of the coolest and most powerful features of interpreted languages like Python. – Boorer

@martineau: in comparison to the alternatives it is very inherently dangerous and insecure. If/when you have no alternative, there is nothing to compare it against so it might be ok to use. – Tenrec

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. – Deckle

I'm with Owen S. Saying that eval is "not dangerous or insecure" is kind of like saying "hand built SQL isn't dangerous or insecure." Sure, there might be rare use cases where it's necessary or times when you simply don't care because security is not an issue for your application, but neither of those negates the fact that eval has the ability to very easily make your application vulnerable to outside attack. "Very easily making my application vulnerable to outside attack" sounds like a reasonable qualification for deeming something "very dangerous and insecure." – Monasticism

Answer 2

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.

Numbersnumbfish

I'm curious what you think a sociopath is going to do with eval? And why that would be a use case that a more innocent programmer might not stumble across as well? – Deckle

@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. – Numbersnumbfish

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. – Deckle

@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. – Numbersnumbfish

Untrusted strings can often be transformed into trusted strings through proper escaping. That's what I meant by "careful". We don't disagree, much as you seem to wish we would. :-) – Deckle

@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. – Numbersnumbfish

Say what you were accepting from the outside world was intended as a string, not arbitrary code. In that case being careful means not splicing the string directly into the code you want to eval – or making sure quotation marks in the string are escaped properly if you do incorporate them into code passed to eval. Or if you're going to take an expression to eval, making sure that what is given to you is of the limited subset of syntax you're willing to accept and have Python work with. – Deckle

@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. – Numbersnumbfish

In conclusion, they can and they aren't, depending on what you're using eval for. I am not going to spend more time discussing it. – Deckle

@OwenS.: "depending on what you're using eval for."? What? Your comments started out talking about eval of data coming from "the outside world" -- an untrusted source -- and being used as scripting attack. Apparently, you're no longer talking about that? If so, then I'm unable to figure out what you don't like about eval. If you're saying eval is not wholly evil, then it appears we agree. – Numbersnumbfish

@Numbersnumbfish Do you realize that you CAN pass strings to eval? eval accepts code objects or strings, which are then parsed as python. If you accept any user input and any of that input goes into the string that is evaled, there is a huge potential for arbitrary code injection. Running code that is composed from user input might sound like a bad idea, and it should, but the point is that not everyone thinks about that. Some who would use eval ARE using user input, and they may even be sanitizing (or THINK they're sanitizing) that input, but may neglect some escape sequence which could inject code. – Strephonn

Answer 3

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]"

Hetaerism

Why is that bad/dangerous? Can't I just execute the same Python code anyway without eval? – Intercut

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. – Clavius

Answer 4

In this case, yes. Instead of

exec 'self.Foo=val'

you should use the builtin function setattr:

setattr(self, 'Foo', val)

Savonarola

Answer 5

It's worth noting that for the specific problem in question, there are several alternatives to using eval:

The simplest, as noted, is using setattr:

def __init__(self):
    for name in attsToStore:
        setattr(self, name, None)

A less obvious approach is updating the object's __dict__ object directly. If all you want to do is initialize the attributes to None, then this is less straightforward than the above. But consider this:

def __init__(self, **kwargs):
    for name in self.attsToStore:
       self.__dict__[name] = kwargs.get(name, None)

This allows you to pass keyword arguments to the constructor, e.g.:

s = Song(name='History', artist='The Verve')

It also allows you to make your use of locals() more explicit, e.g.:

s = Song(**locals())

...and, if you really want to assign None to the attributes whose names are found in locals():

s = Song(**dict([(k, None) for k in locals().keys()]))

Another approach to providing an object with default values for a list of attributes is to define the class's __getattr__ method:

def __getattr__(self, name):
    if name in self.attsToStore:
        return None
    raise NameError, name

This method gets called when the named attribute isn't found in the normal way. This approach somewhat less straightforward than simply setting the attributes in the constructor or updating the __dict__, but it has the merit of not actually creating the attribute unless it exists, which can pretty substantially reduce the class's memory usage.

The point of all this: There are lots of reasons, in general, to avoid eval - the security problem of executing code that you don't control, the practical problem of code you can't debug, etc. But an even more important reason is that generally, you don't need to use it. Python exposes so much of its internal mechanisms to the programmer that you rarely really need to write code that writes code.

Skater

Another way that's arguably more (or less) Pythonic: Instead of using the object's __dict__ directly, give the object an actual dictionary object, either through inheritance or as an attribute. – Savonarola

"A less obvious approach is updating the object's dict object directly" => Note that this will bypass any descriptor (property or other) or __setattr__ override, which might lead to unexpected results. setattr() doesn't have this problem. – Diaphanous

Answer 6

Other users pointed out how your code can be changed as to not depend on eval; I'll offer a legitimate use-case for using eval, one that is found even in CPython: testing.

Here's one example I found in test_unary.py where a test on whether (+|-|~)b'a' raises a TypeError:

def test_bad_types(self):
    for op in '+', '-', '~':
        self.assertRaises(TypeError, eval, op + "b'a'")
        self.assertRaises(TypeError, eval, op + "'a'")

The usage is clearly not bad practice here; you define the input and merely observe behavior. eval is handy for testing.

Take a look at this search for eval, performed on the CPython git repository; testing with eval is heavily used.

Hexyl

Answer 7

When eval() is used to process user-provided input, you enable the user to Drop-to-REPL providing something like this:

"__import__('code').InteractiveConsole(locals=globals()).interact()"

You may get away with it, but normally you don't want vectors for arbitrary code execution in your applications.

Anatto

Answer 8

In addition to @Nadia Alramli answer, since I am new to Python and was eager to check how using eval will affect the timings, I tried a small program and below were the observations:

#Difference while using print() with eval() and w/o eval() to print an int = 0.528969s per 100000 evals()

from datetime import datetime
def strOfNos():
    s = []
    for x in range(100000):
        s.append(str(x))
    return s

strOfNos()
print(datetime.now())
for x in strOfNos():
    print(x) #print(eval(x))
print(datetime.now())

#when using eval(int)
#2018-10-29 12:36:08.206022
#2018-10-29 12:36:10.407911
#diff = 2.201889 s

#when using int only
#2018-10-29 12:37:50.022753
#2018-10-29 12:37:51.090045
#diff = 1.67292

Zarf