Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parsing for safe eval #44

Closed
CMCDragonkai opened this issue Nov 30, 2012 · 4 comments
Closed

Parsing for safe eval #44

CMCDragonkai opened this issue Nov 30, 2012 · 4 comments

Comments

@CMCDragonkai
Copy link

Is there any way for PHP Parser to help in making any user submitted code to be evalled to be more secure?

Like maybe limiting a subset of all functions, or even limiting how the code has to be written in an exact match?

@nikic
Copy link
Owner

nikic commented Dec 15, 2012

Sorry for the late reply, I have no excuse for not answering earlier.

PHP sandboxing was one of my first motivations behind this project, but it is a rather complicated problem if you want to do it by static code analysis / preprocessing. If you are going to evaluate user-submitted code then the only really safe way is to evaluate it in a separate jail (or whever the operating system provides).

Sandboxing through preprocessing is really hard in PHP, because of its dynamic nature. You can easily check that exec(...) is disallowed, but it already is harder to make sure $func() is safe. But that's just the beginning. There are many, many subtle ways in PHP to run something, which are often little-known or easy to overlook. E.g. you could easily miss some particular function taking a callback or some indirect way of executing a call, like with (new ReflectionFunction('exec'))->invoke(...).

So I'm not really sure whether this kind of security checking makes any practical sense as there are just too many points of failure. I find this mostly intriguing from the theoretical point of view. So after I saw this issue I took some old tokenizer-based sandbox code and ported it to use the PHP-Parser. While doing that I found a lot of issues, which I'm trying to resolve. If I manage to figure everything out I'll publish the library. Though as I said, I don't think that it makes sense to use this in practice.

So, to answer the question: Yes, it's possible. But whether it makes sense depends on the specific case. (The narrower the case the easier it should be to properly handle it.)

Again, sorry for the late reply.

@CMCDragonkai
Copy link
Author

Hey nikic, I wrote a sandbox and have it live here http://phpbounce.aws.af.cm

I got through linting, parsing, whitelisting, blacklisting and separate process sandbox. I've switched off whitelisting, but I'm not sure how secure it is. Can you check it out?

BTW, this is a very impressive project for someone who is 17!

@nikic
Copy link
Owner

nikic commented Dec 15, 2012

From a quick look at the code you seem to be using PHP's internal disable_functions and disable_classes features. There should be no way around those short of exploiting PHP itself. Though I obviously don't know how complete the blacklists you are using are.

@CMCDragonkai
Copy link
Author

Indeed, in fact in my whitelist, I went through 4400 functions manually. Anyway thanks for the code it works well.

@nikic nikic closed this as completed Dec 23, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants