Why Is Using 'Eval' a Bad Practice

Why is using 'eval' a bad practice?

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.

Why is using the JavaScript eval function a bad idea?

  1. Improper use of eval opens up your
    code for injection attacks

  2. Debugging can be more challenging
    (no line numbers, etc.)

  3. eval'd code executes slower (no opportunity to compile/cache eval'd code)

Edit: As @Jeff Walden points out in comments, #3 is less true today than it was in 2008. However, while some caching of compiled scripts may happen this will only be limited to scripts that are eval'd repeated with no modification. A more likely scenario is that you are eval'ing scripts that have undergone slight modification each time and as such could not be cached. Let's just say that SOME eval'd code executes more slowly.

Is it a bad practice to use eval(str()) instead of deepcopy() to deepcopy a list of lists?

In case you just got (non-complex) list of lists copying via list comprehension seems to be fastest and more secure than eval:

import timeit
import json
import ujson
import copy


def _eval(l):
return eval(str(l))


def _copy(l):
return copy.deepcopy(l)


def _json(l):
return json.loads(json.dumps(l))


def _ujson(l):
return ujson.loads(ujson.dumps(l))


def _comp(l):
return [x[:] for x in l]


shortList = [[1, 2], [3, 4], [5, 6]]
longList = [[x, x + 1] for x in range(0, 50000)]

for lst in (shortList, longList):
for func in ("_eval", "_copy", "_json", "_ujson", "_comp"):
t1 = timeit.Timer(f"{func}({lst})", f"from __main__ import {func}")
print(f"{func} ran:", t1.timeit(number=1000), "milliseconds")

Out (short list):

_eval ran: 0.009196660481393337 milliseconds
_copy ran: 0.005948461592197418 milliseconds
_json ran: 0.004726926796138287 milliseconds
_ujson ran: 0.0011531058698892593 milliseconds
_comp ran: 0.00045751314610242844 milliseconds

Out (long list):

_eval ran: 16.720303252339363 milliseconds
_copy ran: 7.898970659822226 milliseconds
_json ran: 2.1138144126161933 milliseconds
_ujson ran: 1.2348785381764174 milliseconds
_comp ran: 0.5541304731741548 milliseconds

When is JavaScript's eval() not evil?

I'd like to take a moment to address the premise of your question - that eval() is "evil". The word "evil", as used by programming language people, usually means "dangerous", or more precisely "able to cause lots of harm with a simple-looking command". So, when is it OK to use something dangerous? When you know what the danger is, and when you're taking the appropriate precautions.

To the point, let's look at the dangers in the use of eval(). There are probably many small hidden dangers just like everything else, but the two big risks - the reason why eval() is considered evil - are performance and code injection.

  • Performance - eval() runs the interpreter/compiler. If your code is compiled, then this is a big hit, because you need to call a possibly-heavy compiler in the middle of run-time. However, JavaScript is still mostly an interpreted language, which means that calling eval() is not a big performance hit in the general case (but see my specific remarks below).
  • Code injection - eval() potentially runs a string of code under elevated privileges. For example, a program running as administrator/root would never want to eval() user input, because that input could potentially be "rm -rf /etc/important-file" or worse. Again, JavaScript in a browser doesn't have that problem, because the program is running in the user's own account anyway. Server-side JavaScript could have that problem.

On to your specific case. From what I understand, you're generating the strings yourself, so assuming you're careful not to allow a string like "rm -rf something-important" to be generated, there's no code injection risk (but please remember, it's very very hard to ensure this in the general case). Also, if you're running in the browser then code injection is a pretty minor risk, I believe.

As for performance, you'll have to weight that against ease of coding. It is my opinion that if you're parsing the formula, you might as well compute the result during the parse rather than run another parser (the one inside eval()). But it may be easier to code using eval(), and the performance hit will probably be unnoticeable. It looks like eval() in this case is no more evil than any other function that could possibly save you some time.

About eval is evil and consenting adults

eval is evil because user input gets into it at some point. You don’t (well, shouldn’t) have to be worried about code pretending to not delete all files, because code can do that anyways – tada:

def foo(duck):
duck.quack()

class EvilDuck(object):
os.system('rm -rf /')

def quack(self):
pass

And rm -rf / has a good chance of not working, too. ;)

Basically, “consenting adults” is “trust your code”. eval is “trust all code”. Depending on where you get that code, eval can be fine, but it’s unnecessary 99% of the time, and it can also be hard to guarantee as secure.

Why should exec() and eval() be avoided?

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.

I know eval() is bad practice; is there an alternative for my code which involves getting the __doc__ of all funcs in all files in a folder?

Your variables should be accessible in globals() so you can just

    for name, object in vars(globals()[file]).items():

Is it a bad practice to use eval() in Phonegap?

Using eval() during application development can make your life hard. The evaluated code has access to variables in the scope it was evaluated. This'll easily lead to errors in the program when the new code overwrites your vars. And since the eval'ed code is a string, you won't have so easy to debug this problem. Add the security and performance issues (JS parsers try to pre-compile the code for faster execution, which won't happen if a code hidden inside a string) and you will find that you can make your work easier avoiding eval().

Just in case you really, really need to use the code hidden in a string, use the Function() constructor, with the code string as an argument. That way you will at least avoid the variables overwrite problems.

Example of the difference between eval() and Function():

(function(){
var a = 1;
eval('var a = 2; console.log(a)'); // logs 2
console.log(a); // also logs 2, the a variable is changed
})();

(function(){
var b = 1;
Function('var b = 2; console.log(b)')(); // logs 2
console.log(b); // logs 1, the code string was executed in it's own scope
})();

Is it safe to use Python's eval() in a unit-test for __repr__?

eval is said to be evil because when you eval a string that has been provided by a user, you implicitely allow them to execute arbitrary code.

By here you are using a hard coded string. To be able to execute arbitrary code, an attacker would have to modify the test code or the __repr__ one. In either case that means that they would already have the possibility to execute arbitrary code, regardless of your use of eval.

So my opinion is that there is not security problem with using eval here. At most, you could add a comment for future readers explaining that you have examined that usage and decided that it added no security risk.

Why is `eval` worse than `str2func` to evaluate a function from a string?

I'll put aside the arguments against both methods for a moment, although they are all very valid and a conscientious reader should try to understand them.


I can see two clear differences. For these examples we have to assume you've constructed the input string to the functions in your script, using some variable data, and assume it could be anything (by mistake or otherwise). So we prepare and account for the worst case.

  1. str2func includes some extra checks to make sure what you've passed is a valid function, which might avoid unwanted behaviour. Let's not take the stance of "yeah but you could do it this way to avoid that" and look at an example...

    % Not assigning an output variable
    % STR2FUNC: You've harmlessly assigned ans to some junk function
    str2func('delete test.txt')
    % EVAL: You've just deleted your super important document
    eval('delete test.txt')

    % Assigning an output variable
    % STR2FUNC: You get a clear warning that this is not a valid function
    f = str2func('delete test.txt')
    % EVAL: You can a non-descript error "Unexpected MATLAB expression"
    f = eval('delete test.txt')
  2. Another difference is the subject of about half the str2func documentation. It concerns variable scopes, and is found under the heading "Examine Differences Between str2func and eval".

    [Intro to the function]


    If you use a character vector representation of an anonymous function, the resulting function handle does not have access to private or local functions.



    [Examine Differences Between str2func and eval]


    When str2func is used with a character vector representing an anonymous function, it does not have access to the local function [...]. The eval function does have access to the local function.


So to conclude, you might have use cases where each function is preferable depending on your desired error handling and variable scoping should never use eval or str2func wherever possible, as stated in the other answers.



Related Topics



Leave a reply



Submit