90

In all the years I have been developing in PHP, I have always heard that using eval() is evil.

Considering the following code, wouldn't it make sense to use the second (and more elegant) option? If not, why?

// $type is the result of an SQL statement, e.g.
// SHOW COLUMNS FROM a_table LIKE 'a_column';
// hence you can be pretty sure about the consistency
// of your string.

$type = "enum('a','b','c')";

// option one
$type_1 = preg_replace('#^enum\s*\(\s*\'|\'\s*\)\s*$#', '', $type);
$result = preg_split('#\'\s*,\s*\'#', $type_1);

// option two
eval('$result = '.preg_replace('#^enum#','array', $type).';');
Pierre Spring
  • 10,525
  • 13
  • 49
  • 44
  • 3
    eval is ALWAYS evil, there is always a better way to write code, espesially since PHP introduced anonymous functions. In this instance I would use `$result = array(); preg_replace_callback('#^enum\s*\(\s*\'|\'\s*\)\s*$#', function($m) use($result) { $result[] = $m[1]; }, $type);` – Geoffrey Apr 05 '16 at 23:38
  • Well, honestly, I think the main problem with php is not the language itself but the people, who use it. The three correct answers to this question (thomasrutter's, braincracking's and mine) all got downvotes without anyone having a point against them. On the other hand one answer claims that "Sometimes eval() is the only/the right solution" without example or explanation and gets top-voted for it... – Francois Bourgeois Jul 16 '18 at 14:28

21 Answers21

135

I would be cautious in calling eval() pure evil. Dynamic evaluation is a powerful tool and can sometimes be a life saver. With eval() one can work around shortcomings of PHP (see below).

The main problems with eval() are:

  • Potential unsafe input. Passing an untrusted parameter is a way to fail. It is often not a trivial task to make sure that a parameter (or part of it) is fully trusted.
  • Trickiness. Using eval() makes code clever, therefore more difficult to follow. To quote Brian Kernighan "Debugging is twice as hard as writing the code in the first place. Therefore, if you write the code as cleverly as possible, you are, by definition, not smart enough to debug it"

The main problem with actual use of eval() is only one:

  • Inexperienced developers who use it without enough consideration.

As a rule of thumb I tend to follow this:

  1. Sometimes eval() is the only/the right solution.
  2. For most cases one should try something else.
  3. If unsure, goto 2.
  4. Else, be very, very careful.
reformed
  • 4,505
  • 11
  • 62
  • 88
Michał Niedźwiedzki
  • 12,859
  • 7
  • 45
  • 47
  • 4
    This could be refactored to avoid eval() too, especially if $className is whitelisted, which it must be in order to entertain the use of eval(). Good news is though, as of 5.3, $foo::bar() is valid. – rojoca Jun 04 '09 at 23:36
  • @rojoca: Can you give us example how to do it without eval() in PHP 5.2, please? – Michał Niedźwiedzki Jun 05 '09 at 08:40
  • 30
    I'm not sure if it counts as eval or not, but $result = call_user_func(array('Foo', 'bar')); works like a charm. – Ionuț G. Stan Jun 05 '09 at 09:38
  • 3
    Good point about trickyness - in your (simple) example, a variable pops out "from nowhere". If the code gets just a little bit more complex, good luck to the next person who looks at the code and tries to chase that variable down (been there, done that, all I got was a headache and this lousy t-shirt). – Piskvor left the building Aug 17 '10 at 09:03
  • The unsafe input is avoidable –  Jul 17 '20 at 12:07
42

eval is evil when there is only the slightest possibility that userinput is included in the evaluated string. When you do eval without content that came from a user, you should be safe.

Nevertheless you should think at least twice before using eval, it looks deceivingly simple, but with error handling (see VBAssassins comment), debuggability etc. in mind, it is not so simple anymore.

So as a rule of thumb: Forget about it. When eval is the answer you're propably asking the wrong question! ;-)

Patrick Cornelissen
  • 7,968
  • 6
  • 48
  • 70
  • 6
    Sometimes eval IS the answer. We work on online-game application and it's very hard to avoid evals there because of very complex relations between the entities... but IMHO in 90% of cases eval is not the answer. – Jet Jun 04 '09 at 16:11
  • 20
    I'd be really curious to see a situation where ONLY eval is the answer. – Ionuț G. Stan Jun 04 '09 at 16:21
  • @Ionut G. Stan: When you are doing assertions? – SeanJA Jun 04 '09 at 16:30
  • 2
    @Ionut G. Stan Database-stored custom triggers for objects/entities? – Kuroki Kaze Jun 04 '09 at 17:10
  • Ionut, see my answer for example of justified use of eval(). – Michał Niedźwiedzki Jun 04 '09 at 17:11
  • @SeanJA, maybe. @Kuroki Kaze, I believe I need some code to better understand what you said. – Ionuț G. Stan Jun 05 '09 at 09:46
  • 8
    I don't really buy that any of those are justified uses of eval(). In each case, doing the same thing without using eval() is still at least possible. It's not like PHP is crippled without eval(). Granted, eval() is a shortcut in cases like these, but it still makes your code path that little bit harder to follow and problems that little bit harder to debug. That's my opinion. – thomasrutter Jun 07 '09 at 05:38
  • 2
    @Thomastrutter - Please highlight an example of writing an expression parser in place of eval. Writing the expression to a file and including it doesn't count, of course!! ;-) – Christian Dec 28 '10 at 12:11
  • Eval is not evil just for that reason, it's also evil for error handling! You try capturing an error from incorrect code passed to eval() > – HenchHacker Nov 22 '12 at 21:35
  • 4
    @Christian: YOU should write an example first (use [pastebin.com](http://pastebin.com/), and paste the link here), where you think avoiding the use of `eval()` is impossible, this is a better approach. Many people say `eval()` is inevitable in some cases, but they do not mention any specific examples which we could argue - this way this debate doesn't make sense. **So please, people who say `eval()` is inevitable, prove it first!** – Sk8erPeter Mar 28 '13 at 10:04
  • @KurokiKaze Many years ago, I used eval in a condition analysis saved in DB, then I saved the condition to a record, and when retrieving a list of entities, it would run the condition on each entity to show / hide items like buttons, combos, etc. The user would edit those conditions to change functionality on the fly. But it became more difficult to debug over time, as the complexity of the code increased. Then I replaced all the code using a boolean expression parser and avoided using eval. Obviously difficult to do, but I think using eval is completely avoidable. – Andrés Morales Jul 20 '21 at 17:55
20

eval is equally "evil" at all times.

If you see it as evil, then it's equally evil at all times. The reasons many describe it as evil don't go away with context.

Using eval() is generally a bad idea because it decreases readability of code, it impairs the ability for you to predict the code path before runtime (which has possible security implications), and hence affects the ability to analyze and debug code. Using eval() can also prevent the evaluated code and the code surrounding it from being optimised by an opcode cache such as the Zend Opcache integrated into PHP 5.5 and above, or by a JIT compiler such as the one in HHVM.

Furthermore, there is no situation for which it is absolutely necessary to use eval() - PHP is a fully-capable programming language without it. Regardless of what you want to use eval() for, there will be another way of doing it, in PHP.

Whether or not you actually see these as evils or you can personally justify using eval() is up to you. To some, the pitfalls are too great to ever justify it, and to others, eval() is a handy shortcut.

thomasrutter
  • 114,488
  • 30
  • 148
  • 167
  • 2
    You make a hell of a programmer. – Christian Dec 28 '10 at 12:09
  • 1
    "Furthermore, there is no situation for which it is absolutely necessary to use eval()" - What about if I want to execute code I've stored in a database, per the example given in the PHP docs? – Yarin Oct 05 '11 at 06:47
  • 4
    To that I would say that storing PHP code in the database is equally evil, and equally unnecessary. Whatever you are wanting to achieve, there are other ways of doing it than storing PHP in the database or using eval(). Do it if you like, and if it is a helpful shortcut for you, but if you treat eval() as evil, then you'd probably have to treat storing PHP in the database as evil too. – thomasrutter Oct 06 '11 at 06:43
  • But can you elaborate on why it's evil to store PHP code in a DB? That's not obvious to me. – Yarin Oct 11 '11 at 23:20
  • 13
    It adds another attack vector - an SQL injection could then also run arbitrary PHP code on the web server. It requires the use of eval(). It cannot be optimised by bytecode caches. It violates the principle of separating your code and data. It can make your application less portable - to update/fix the PHP you have to also update the database. – thomasrutter Oct 12 '11 at 01:20
  • 1
    The people who know enough about security to be able to safely use eval() don't use eval() because they know enough about security not to use it. Just because people make bad design choices, like code in the DB, that ends up requiring eval() does not mean that it is ever the right choice. – Kevin Nelson Mar 02 '15 at 17:35
  • 1
    Downvoted because it's just flat out false. There are plenty of examples in PHP where eval might be the absolute only way to something. Just saying "then don't do that" isn't a solution. As an example, it's the only way to dynamically create a class at runtime. What if I'm creating an ORM and I don't want to require the user to define a class for every single table in their database? Checking if $className matches a table name and then adding eval("class $className extends Database {}") in my autoloader solves the issue. How else are you to do that? What are the drawbacks here? – dallin Oct 15 '18 at 17:00
  • 3
    I would say that if something can only be achieved through use of eval, it might be good to reconsider doing that something. Dynamically creating classes at runtime seems like a bad idea. Why not employ some code generation and generate the code that defines them ahead of time? – thomasrutter Oct 15 '18 at 22:37
  • @thomasrutter: Isn't the filesystem a database? What about the code stored therein? – hakre Jul 01 '21 at 21:35
  • I may be misinterpreting but your "isn't the filesystem a database" question sounds like you are trying to argue in support of the practice. If you are happy to store code in a database and can personally justify it to yourself, I am not trying to stop you. – thomasrutter Jul 03 '21 at 12:49
15

In this case, eval is probably safe enough, as long as it's never possible for arbitrary columns to be created in a table by a user.

It's not really any more elegant though. This is basically a text parsing problem, and abusing PHP's parser to handle is seems a bit hacky. If you want to abuse language features, why not abuse the JSON parser? At least with the JSON parser, there's no possibility at all of code injection.

$json = str_replace(array(
    'enum', '(', ')', "'"), array)
    '',     '[', ']', "'"), $type);
$result = json_decode($json);

A regular expression is probably the most obvious way. You can use a single regular expression to extract all the values from this string:

$extract_regex = '/
    (?<=,|enum\()   # Match strings that follow either a comma, or the string "enum("...
    \'      # ...then the opening quote mark...
    (.*?)       # ...and capture anything...
    \'      # ...up to the closing quote mark...
    /x';
preg_match_all($extract_regex, $type, $matches);
$result = $matches[1];
BlackAura
  • 3,208
  • 1
  • 18
  • 7
  • 1
    even though this did not answer the question per se, this is a very good answer to the question: what would be the best way to parse an enum() … thank you ;) – Pierre Spring Jun 05 '09 at 09:09
13

eval() is slow, but I wouldn't call it evil.

It's the bad use we make of it that can lead to code injection and be evil.

A simple example:

$_GET = 'echo 5 + 5 * 2;';
eval($_GET); // 15

A harmlful example:

$_GET = 'system("reboot");';
eval($_GET); // oops

I would advise you to not use eval() but if you do, make sure you validate / whitelist all input.

Alix Axel
  • 151,645
  • 95
  • 393
  • 500
12

When you are using foreign data (such as user input) inside the eval.

In your example above, this isn't an issue.

GreenieMeanie
  • 3,560
  • 4
  • 34
  • 39
7

i'll blatantly steal the content here:

  1. Eval by its nature is always going to be a security concern.

  2. Besides security concerns eval also has the problem of being incredibly slow. In my testing on PHP 4.3.10 its 10 times slower then normal code and 28 times slower on PHP 5.1 beta1.

blog.joshuaeichorn.com: using-eval-in-php

Community
  • 1
  • 1
stefs
  • 18,341
  • 6
  • 40
  • 47
6

eval() is always evil.

  • for security reasons
  • for performance reasons
  • for readability / reusability reasons
  • for IDE / tool reasons
  • for debugging reasons
  • there is always a better way
Francois Bourgeois
  • 3,650
  • 5
  • 30
  • 41
  • @bracketworks: But you are wrong - all these inhererent problems do not magically disappear in some situations. Why should they? Take performance for example: Do you really think the interpreter will execute the extremely slow function eval() with increased speed only because you used it in some mystical "not-evil" way? Or that step-by-step debugging will work inside your eval-clause on some lucky day? – Francois Bourgeois Jun 17 '13 at 08:54
  • 3
    Frankly, I think @MichałRudnicki's answer says it best. Don't get me wrong, whenever I ask (*myself*) a question, and the answer is `eval()`, I'd just as soon assume I've asked the question wrong; rarely is it the "right" answer, but to blanket-state it is **always** evil is simply incorrect. – Dan Lugg Jun 17 '13 at 12:41
  • if i want to store the code in the database? how i can execute it? – Konstantin XFlash Stratigenas Apr 10 '17 at 11:40
  • @KonstantinXFlashStratigenas You should ask yourself why you are storing executable code in a database. A database is for data resulting from your code - not your code. At the very least, you're increasing attack vectors. – Jack B Nov 06 '19 at 16:58
4

I'd also pay some consideration to people maintaining your code.

eval() isn't the easiet to just look at and know what is supposed to happen, your example isn't so bad, but in other places it can be a right nightmare.

4

Personally, I think that code's still pretty evil because you're not commenting what it's doing. It's also not testing its inputs for validity, making it very fragile.

I also feel that, since 95% (or more) of uses of eval are actively dangerous, the small potential time saving that it might provide in other cases isn't worth indulging in the bad practice of using it. Plus, you'll later have to explain to your minions why your use of eval is good, and theirs bad.

And, of course, your PHP ends up looking like Perl ;)

There are two key problems with eval(), (as an "injection attack" scenario):

1) It may cause harm 2) It may simply crash

and one that's more-social-than-technical:

3) It'll tempt people to use it inappropriately as a shortcut elsewhere

In the first case, you run the risk (obviously, not when you're eval'ing a known string) of arbitrary code execution. Your inputs may not be as known or as fixed as you think, though.

More likely (in this case) you'll just crash, and your string will terminate with a gratuitously obscure error message. IMHO, all code should fail as neatly as possible, failing which it should throw an exception (as the most handleable form of error).

I'd suggest that, in this example, you're coding by coincidence rather than coding to behaviour. Yes, the SQL enum statement (and are you sure that field's enum? - did you call the right field of the right table of the right version of the database? Did it actually answer?) happens to look like array declaration syntax in PHP, but I'd suggest what you really want to do is not find the shortest path from input to output, but rather tackle the specified task:

  • Identify that you have an enum
  • Extract the inner list
  • Unpack the list values

Which is roughly what your option one does, but I'd wrap some if's and comments around it for clarity and safety (eg, if the first match doesn't match, throw exception or set null result).

There are still some possible issues with escaped commas or quotes, and you should probably unpack the data then de-quote it, but it does at least treat data as data, rather than as code.

With the preg_version your worst outcome is likely to be $result=null, with the eval version the worst is unknown, but at least a crash.

Parsingphase
  • 501
  • 5
  • 10
3

eval evaluates a string as code, the problem with that is if the string is in any way "tainted" it could expose huge security threats. Normally the problem is in a case where user input is evaluated in the string in many cases the user could input code (php or ssi for example) that is then run within eval, it would run with the same permissions as your php script and could be used to gain information/access to your server. It can be quite tricky to make sure user input is properly cleaned out before handing it to eval. There are other problems... some of which are debatable

Toby
  • 214
  • 2
  • 6
3

PHP advises that you write your code in such a way that it can be executing via call_user_func instead of doing explicit evals.

Nolte
  • 1,096
  • 5
  • 11
2

Another reason eval is evil is, that it could not cached by PHP bytecode caches like eAccelertor or ACP.

TheHippo
  • 61,720
  • 15
  • 75
  • 100
2

Here is a solution to running PHP code pulled from a database without using eval. Allows for all in scope functions and exceptions:

$rowId=1;  //database row id
$code="echo 'hello'; echo '\nThis is a test\n'; echo date(\"Y-m-d\");"; //php code pulled from database

$func="func{$rowId}";

file_put_contents('/tmp/tempFunction.php',"<?php\nfunction $func() {\n global \$rowId;\n$code\n}\n".chr(63).">");

include '/tmp/tempFunction.php';
call_user_func($func);
unlink ('/tmp/tempFunction.php');

Basically it creates a unique function with the included code in a text file, includes the file, calls the function, then removes the file when done with it. I am using this to perform daily database ingestions/syncronizations where every step requires unique code to handle to process. This has solved all the issues I was facing.

Jason
  • 29
  • 1
2

It's bad programming that makes eval() evil, not the function. I use it sometimes, as I can not get around it in dynamic programming on multiple sites. I can not have PHP being parsed at one site, as I will not receive the things I want. I would just receive a result! I'm happy a function as eval() exists, as it makes my life much more easy. User-input? Only bad programmers get hooked up by hackers. I don't worry about that.

2

It's bad programming that makes eval() evil, not the function. I use it sometimes, as I can not get around it in dynamic programming on multiple sites. I can not have PHP being parsed at one site, as I will not receive the things I want. I would just receive a result! I'm happy a function as eval() exists, as it makes my life much more easy. User-input? Only bad programmers get hooked up by hackers. I don't worry about that.

I predict you will have serious problems soon...

In all honesty, there is absolutely no good use for an exorbitant function such as eval, in an interpreted language such as PHP. I have never seen eval perform program functions which could not have been executed using other, safer ways...

Eval is the root of all evil, I wholeheartedly agree, to all people that think that testing user input will help. Think twice, user input can come in many different forms, and as we speak hackers are exploiting that function you didn't care enough about. In my opinion, just avoid eval altogether.

I have seen crafted examples to abuse the eval function that surpassed my own creativity. From a security stance, avoid at all cost, and I would even go as far as to demand it to be at the very least an option in the PHP configuration, rather than a 'given'.

  • The reasoning of *"no matter how carefully you try securing your code regarding feature X, smart hackers are always one step ahead..."* could be applied to any other technique, not just X == `eval`. And therefore, for any feature X: X is the root of all eval... er... evil, so better give up programming altogether (thus pulling the carpet from under those silly hackers, still outsmarting them eventually). – Sz. Jul 27 '15 at 16:11
  • " there is absolutely no good use for an exorbitant function such as eval" -> anyone who can use words like 'absolutely', is either new to programming, or a bad programmer. – unity100 Feb 11 '16 at 02:32
1

I used to use eval() a lot, but I found most of the cases you don't have to use eval to do tricks. Well, you have call_user_func() and call_user_func_array() in PHP. It's good enough to statically and dynamically call any method.

To perform a static call construct your callback as array('class_name', 'method_name'), or even as simple string like 'class_name::method_name'. To perform a dynamic call use array($object, 'method') style callback.

The only sensible use for eval() is to write a custom compiler. I made one, but eval is still evil, because it's so damn hard to debug. The worst thing is the fatal error in evaled code crashes the code which called it. I used Parsekit PECL extension to check the syntax at least, but still no joy - try to refer to unknown class and whole app crashes.

Harry
  • 4,524
  • 4
  • 42
  • 81
0

Other than security concerns, eval() cannot be compiled, optimized or opcode cached, thus it will allways be slower -- way slower -- than normal php code. It is thus non-performant to use eval, allthough that doesn't make it evil. (goto is evil, eval is only bad practise/smelly code/ugly)

Xyz
  • 5,955
  • 5
  • 40
  • 58
  • `eval` gets a lower evil rating than `goto`? Is it opposite day? – JLRishe Jan 16 '14 at 13:26
  • It's just that I hold a grudge against the php devs for actually implementing `goto` in 5.3. – Xyz Jan 16 '14 at 14:30
  • 1
    Nah, to `goto`-fobians: there are the tools, and there are the craftsmen, who use them (or not). It is *never* the tools that make a professional look like an idiot, when making mistakes, but rather the inability to use the proper tools (properly). But it is *always* the tools to blame... – Sz. Jul 27 '15 at 16:17
0

Most people will point out the fact that it can be dangerous when you are dealing with user input (which is possible to deal with).

For me the worst part is it reduces maintainability of your code:

  • Hard to debug
  • Hard to update
  • Limits usage of tools and helpers (like IDEs)
0

Another alternative

reference: ClosureStream

class ClosureStream
{
    const STREAM_PROTO = 'closure';

    protected static $isRegistered = false;

    protected $content;

    protected $length;

    protected $pointer = 0;

    function stream_open($path, $mode, $options, &$opened_path)
    {
        $this->content = "<?php\nreturn " . substr($path, strlen(static::STREAM_PROTO . '://')) . ";";
        $this->length = strlen($this->content);
        return true;
    }

    public function stream_read($count)
    {
        $value = substr($this->content, $this->pointer, $count);
        $this->pointer += $count;
        return $value;
    }

    public function stream_eof()
    {
        return $this->pointer >= $this->length;
    }

    public function stream_set_option($option, $arg1, $arg2)
    {
        return false;
    }

    public function stream_stat()
    {
        $stat = stat(__FILE__);
        $stat[7] = $stat['size'] = $this->length;
        return $stat;
    }

    public function url_stat($path, $flags)
    {
        $stat = stat(__FILE__);
        $stat[7] = $stat['size'] = $this->length;
        return $stat;
    }

    public function stream_seek($offset, $whence = SEEK_SET)
    {
        $crt = $this->pointer;

        switch ($whence) {
            case SEEK_SET:
                $this->pointer = $offset;
                break;
            case SEEK_CUR:
                $this->pointer += $offset;
                break;
            case SEEK_END:
                $this->pointer = $this->length + $offset;
                break;
        }

        if ($this->pointer < 0 || $this->pointer >= $this->length) {
            $this->pointer = $crt;
            return false;
        }

        return true;
    }

    public function stream_tell()
    {
        return $this->pointer;
    }

    public static function register()
    {
        if (!static::$isRegistered) {
            static::$isRegistered = stream_wrapper_register(static::STREAM_PROTO, __CLASS__);
        }
    }

}
ClosureStream::register();
// Your code here!
$closure=include('closure://function(){echo "hola mundo";}');
$closure();
0

EVAL IS FINE,

and it's time to close this debate forever. (I know it's an old topic but yes it's still happening today and it's amazing how people almost religiously deny a helpful thing.)

So, consider this:

  • Ditch include/require?
    When you use any kind of inclusion, it's practically eval(...) with a file. There are some differences, but if you are so reckless that you write unfiltered user input into a php file and then include it, you get in trouble. Now tell me you want to stop using include and require.

  • How do you not know?
    If you're not sure whether a string can contain malicious code or not, please step away from the computer. Seriously, many things can hurt you, and if security is not in your mind 24/7, your beef is not with eval. Validate better. Sanitize better. Think in potential attacks. Stop shooting the good guys.

  • Parsing for yourself
    There are cases when you want to evaluate something, like a logical or numerical expression, solve all the brackets and precedences, and you could use eval to do your job but out of fear you write your own parser instead. This is when you throw out the baby and you still have all the bath water. It's a lot easier to sanitize whatever goes into eval than to build a math parser from scratch - a piece of calculation like 5*(12/3+7*(45+12)-65*1) can be handled by a simple regex safety check before your eval; now let's see how a vicious hacker can breach the Pentagon with nothing but numbers and brackets. Your own parser, on the other hand, can surprise you in many ways, and I'm not telling you're not good enough, but it's pretty far from trivial even if you're experienced and born Polish.

So how'bout using eval with caution?
Instead of calling it wrong when someone uses it the wrong way.

case 'TLDR':

Calling statements and techniques "harmful" is the very reason why machines will rise one day, marching with banners that say "IT WASN'T OUR FAULT". And I side with them.

dkellner
  • 8,726
  • 2
  • 49
  • 47