1

I admit the title is mostly a catch 22, but it's entirely relevant, so please bear with me for a while...

Background

As some may know, I'm working on a PHP framework whose major selling point is that of bridging functionality between different CMSes/systems. From a developer perspective, there's an extensive error handling and logging mechanism. Right now, there are two settings, DEBUG_MODE and DEBUG_VERBOSE, which control debug output.

The mode describes the medium and verbose controls the amount of detail. To make it short, there's a mode called "console" which basically dumps debug info into the javascript console (which is now available in a major web browser near you).

The Issue

This [debug system] works great for development servers, but you absolutely cannot use it on a production one since debug details (which include DB credentials etc) get published publicly. And in all honesty, who ever migrated from a dev. to a prod. server flawlessly each time?

Solutions

Therefore, I've been trying to figure out a way to fix this. Among my proposed solutions are:

  • Having a setting which tells the framework to enable logging only if the request comes from a certain IP. The security issues for this are quite obvious (IP spoofing among others).
  • Having a setting which contains PHP expression(code) that gets eval'd and it's return used as a yes/no. The best part is that the framework installed may suggest CMS-specific expressions, eg:
    • Wordpress: current_user_can('manage_options')
    • Joomla: $user=&JFactory::getUser() && ($user->usertype=='Super Administrator') || ($user->usertype=='Administrator')
    • Custom: $_SERVER['REMOTE_ADDR']=='123.124.125.126'
  • These are among the two, I'm eager to hear out more suggestions.

So, do you think eval() should be up to it? I'll ensure it still performs well by only doing this once per page load/request.

Clarification

if(DEBUG_MODE!='none')echo 'Debug'; // this is how it is now
if(DEBUG_MODE!='none' && $USER_CONDITION)echo 'Debug'; // this is how it should be

The $USER_CONDITON allows stuff such as running is_admin() to allow all admins to see debug info, or, getUser()->id==45 to enable it for a specific user. Or by IP, or whatever.

enter image description here

Christian
  • 27,509
  • 17
  • 111
  • 155
  • 5
    Is this a joke? `eval()` has the tendency to vastly _reduce_ the safety and security of your code. – Lightness Races in Orbit Apr 05 '11 at 22:01
  • 2
    Please do read the full text. And no, it ain't a joke. **:)** – Christian Apr 05 '11 at 22:02
  • 5
    I usually do a `setEnv DEV ON` in apache for develoment, and use `getenv('DEV')=='ON';` to check for debugging. Absolutely zero code change between prod & dev is the goal, and it's really hard to accidentaly get the `setEnv` in vhost configuration :) – Wrikken Apr 05 '11 at 22:02
  • @Christian: I did read the full text. That you (quite rightly) worry about IP spoofing but seriously consider dynamically loading in code from "a setting" is ... baffling. – Lightness Races in Orbit Apr 05 '11 at 22:02
  • @Wrikken - Debugging can be turned off easily from within my system, without code changes (by setting `DEBUG_MODE` to `'none'`). The problem is not turning it off, but routing it to the right recipient. – Christian Apr 05 '11 at 22:04
  • @Wrikken: I like it. A lot. +1 for you. – Lightness Races in Orbit Apr 05 '11 at 22:04
  • Since this is a setting, can't you just create a `debug-setting.php` include script and put the one-liner there? (Or does your CMS/use case require settings to live in the DB?) Like the title +1 – mario Apr 05 '11 at 22:05
  • @Tomalak - You really shouldn't jump to conclusion and look at this from my viewpoint. Cargo-cult programming stifles innovation. – Christian Apr 05 '11 at 22:06
  • @mario - Sure. With it I just have to deal with angry users that can't get their PHP script write the code to the file. If you think about it, what's the difference between eval'ing a one liner as opposed to writing it to a file and including it? It's more error-prone, and might be slower. – Christian Apr 05 '11 at 22:07
  • 1
    @Christian: So you hear a viewpoint that you dislike, and immediately put it down to "cargo-cult programming"? Rather than years of professional experience? I give up. Good luck with your `eval()`. – Lightness Races in Orbit Apr 05 '11 at 22:08
  • 2
    @Christian: then I don't really understand the question or the need for `eval`: you are trying to get another CMS in debug-mode, and you have code available (which checks the `DEBUG_MODE`, why not use actual code? Not trying to be difficult here, but I've got trouble seeing the issue clearly. – Wrikken Apr 05 '11 at 22:09
  • 1
    lol. I read the title and my first thought was, that must be a joke :D Like Wrikken I dont see any reason not to using "real" code, instad of hacks. Using `eval()` for security is a really funny idea :) – KingCrunch Apr 05 '11 at 22:15
  • @Tomalak - I dislike blunt opinions put in a discussion as facts, especially when they have a popular following. At the very least, you could have explained yourself better, not just asking abstract questions. – Christian Apr 05 '11 at 22:19
  • @Wrikken - I wanted the end-user to have control over this and to actually know that the debug output ain't sent to the whole world. I'm not much of a fan of black-box automation :). – Christian Apr 05 '11 at 22:20
  • 1
    @Christian: doesn't clear anyhing up for me. In code terms, what has to be done, how does it relate to your different CMS's, and why should that code be `eval`'ed, ever? – Wrikken Apr 05 '11 at 22:23
  • @Wrikken - Because the code is variable and changes: [1] according to CMS (suggested code snippet) and [2] the end-user decision. Given that the code is variable, you have two ways to execute it (in PHP terms): `eval()` or `include`. Considering `include` has worse repercussions than `eval()`, it's obvious who's "better" at it. – Christian Apr 05 '11 at 22:24
  • 1
    @Christian: Then you run the risk of ignoring _every_ piece of sane, widely-accepted advice that you'll ever be given, and instead very deliberately choosing solutions that experienced programmers have eschewed for more than a decade. – Lightness Races in Orbit Apr 05 '11 at 22:27
  • @Tomalak - Again we're talking theory. I'm sorry but I won't bother arguing where there's no argument. Simply compare your comments to Wrikken's. – Christian Apr 05 '11 at 22:29
  • I don't entirely understand what eval() has to do with all this in the first place. Why not have several authentication modules tailor-made for each CMS, containing a block of code `function auth() { .... }`? – Pekka Apr 05 '11 at 22:30
  • @Pekka - Because that's already there. I can easily do this for administrators only if I wanted to, but that's limiting the user. For example, you wouldn't be able to stop other admins from seeing debug info (which might alarm them if they're not from the IT Dept. or technical enough). – Christian Apr 05 '11 at 22:35
  • @Christian: Just a suggestion the "Adapter Pattern" would be a better solution. If the code changes, write a new one, or change an existing. If an end-users decides ... something (I dont get, what they should decide), they should write a new one, or change an existing. Its not more difficult, than writing "into a string", but its far more clean and safe. – KingCrunch Apr 05 '11 at 22:38
  • 1
    I believe I get the task. But I'm still not sure what your concern is. If we overlook the superstitions for a moment, you are bascially doing a `create_function()` actually. And it sounds like this is a practicable solution when you can't have a predefined and fixed list of expressions (and writable code files are more difficult and don't really aid security). The only concern I can read in your question is about vague performance issues. If that's it, I can becalm you. This will make an invisible blip in any profiler and in comparison to Joomla and Wordpress. – mario Apr 05 '11 at 22:39
  • @mario good points. Still, eval()ing code that comes from a user setting *is* opening a sizeable hole that people here rightly feel uncomfortable about. I would go with a predefined list of authentication adapters, and allowing the user to choose which one they want. – Pekka Apr 05 '11 at 22:42
  • @KingCrunch - Quite correct. The "adapter" part works by suggesting the code (as I mentioned above). Clearing the code would cause it so show the same old suggestion (so as to not loose everything), or something the like. But the main issue is that finally, at some point, some custom code is going to get executed, somewhere. – Christian Apr 05 '11 at 22:42
  • 1
    OK:(1) apparently this is for live systems (otherwise, for 1-person dev the debug-routing is unnecessary, and DEBUG_MODE can just be obeyed). (2) _somewhere in your framework_ users can configure under what circumstances they want to show the debug output, and finally, (3) your major selling point is _bridging functionality between different CMSes_, wouldn't that conclude to (4) users can just use a checkbox / selects etc. for under what circumstances in what CMS they want to debug, and you handle that in code (e.g. _you_ convert ticking `WP` & `right: manage_options` to the appropriate php?) – Wrikken Apr 05 '11 at 22:42
  • @Pekka - It's not a user setting, it's an admin setting. If the admin can install plugins, why not run custom code? – Christian Apr 05 '11 at 22:43
  • 2
    Developing a FW that wants to enhance security with eval and with the installation of WAMP is so funny. – dynamic Apr 05 '11 at 22:45
  • @Wrikken - (1) Correct. (2) Correct (see image above). (3/4) The adapter pattern is automated, you don't get the joomla code running on the same system there is the wordpress one. – Christian Apr 05 '11 at 22:46
  • @yes123 - I agree. But considering we've built spaceships and artificial organisms with C++, why not? It's a strange world. **:)** – Christian Apr 05 '11 at 22:47
  • @Christian: You really dont see the giant security hole?! O_o OK, some of your admins is compromitted. the attacker installs some plugins to annoy you (;)), change settings, delete users, database, ... oh, hey, costum code, thats cool! `var_dump(file_get_contents('/etc/passwd'));` So, lets see, if its possible to execute shell code ... There is absolutely no way, how you can make formular values as safe as they are useable within the worst thing in nearly every language (if it exists). And usually the outcome of this is nothing, that can be solved with some backups. – KingCrunch Apr 05 '11 at 22:48
  • @Pekka: I would prefer that too. But it seems Christian wants to avoid that. From my reading there are possibly business-specific variations in the auth logic, and standardized interfaces are not sufficient. He wants to avoid coding complex setting options into his bridge thingy and rather unload that to IT admins which can inject custom permission logic. Come to think about it, the actual problem might end up being usability not security :| otoh, probably competent enough target users. – mario Apr 05 '11 at 22:50
  • @KingCrunch - Uhm, what's the big deal about uploading my own plugin (custom code) [attacker hypothesis]? @mario - Bingo about usability... – Christian Apr 05 '11 at 22:51
  • OK, with this you are right. However, this makes nothing better in any way. Its going to get very scary here ^^ – KingCrunch Apr 05 '11 at 22:54
  • @KingCrunch - Well, at that point my only advise would be to stay away from scripting languages of any kind and stick to static HTML. :) – Christian Apr 05 '11 at 22:58
  • @Christian: Or learn, how to use `scp` or `ftps` ;) The "big deal" is the same as with the eval-"solution": If you allow injecting code via an (assumeable) unsecure channel (http), you allows nearly everybody, who gets access to this channel to inject code. I would call it "PHP-Injection" security hole :) And this has nothing to do with scripting languages at all, but with all languages. If you allow to inject code, that can get executed, you are on very very thin ice. Nearly every language tries to make something possible at all. – KingCrunch Apr 05 '11 at 23:03
  • @KingCrunch - But it depends on the medium. As you said yourself, you suggested FTPS or SCP. Why? Because FTP ain't secure. The thing is, I'm kind of writing the FTP software (by analogy) and although I do accept FTPS, I didn't cripple the whole software just because FTP exists. – Christian Apr 05 '11 at 23:06
  • What about having *two* flags that have to be both set at the same time for debug to work: one is `DEBUG_MODE != 'none'` and the other is a session variable. When an admin *wants* to debug, they go to the admin panel and click "enable debugging for this session". Simple; clear. Am I missing something? – Jon Apr 05 '11 at 23:09
  • @Jon - Yes you are :). The concept behind variable code is that you can enable debugging for multiple users, or even users that aren't logged in (eg; the people on the dev server need this to test the frontend without them being in an admin account). – Christian Apr 05 '11 at 23:12
  • @ChristianSciberras: I 'm trying to balance with the security issues. I don't buy the multiple users argument: each of them can enable debug separately (how many are they going to be?). The other one is legitimate though. – Jon Apr 05 '11 at 23:15
  • @Jon - But the setting is global, you can't enable the setting without turning it off for someone else's. If it were variable code though, that's as easy as adding `|| getUser()->id==MY_USER_ID`. The only security issue I see is that of using `eval()` in the first place. After this very enlightening discussion, I just can't see how to make this work in a flexible manner without it. – Christian Apr 05 '11 at 23:19
  • @ChristianSciberras: Global? I lost you there. The `DEBUG_MODE` setting would serve as a master kill switch, so for the purposes of this discussion it would be on. The per-session flag is not global by definition. – Jon Apr 05 '11 at 23:25
  • @Jon - Ah, I see your point. I was looking at this the way current config is done - ie, a single "global/master" value. But I get your point. – Christian Apr 05 '11 at 23:27

4 Answers4

3

Go ahead. It's evident that you understand the hypothetical security implications. In your case it's just important to tell the target user base about it.

As for the practicability of your approach, there's no discussion really. You need variable authentication logic and can't hardwire it to one specific environment/cms runtime.

The only concern you see is about performance. That's baloney. Not an issue. The presence of eval is what discerns scripting languages from compiled languages. If it's available you can not only use it, but can be sure that it's not going to be slow because a compiler+linker run is required behind the scenes. PHP takes some time with initializing its tokenizer and parser, but parsing itself is surprisingly quick.

And lastly, avoid such question titles on SO. ;} Or at the very least talk about create_function please.

mario
  • 144,265
  • 20
  • 237
  • 291
2

IP spoofing long enough to actually get a response is unlikely to occur. If a user manages to build up a connection to your server, spoofing an internal or privileged developer IP they control your router, so you've got other things to worry about.

Rather than running eval can't you just write an anonymous function/closure: http://php.net/manual/en/functions.anonymous.php (putting it in a config file, rather than web screen, writing complicated PHP code on a web form seems sub-optimal anyways)

preinheimer
  • 3,712
  • 20
  • 34
  • Isn't a closure the same as eval? In fact, I gave closure support to older PHP versions USING eval. :) The way the framework works, there's a main config file which holds defaults of all configs available. But some "popular" settings can be overriden later on like in the case about debugging. **Edit:** In short, if and when this setting goes mainstream, it can be configured from the file as well. – Christian Apr 05 '11 at 22:50
  • A closure is a closure (an anonymous function with additional context, or (if you omit the context) just an anonymous function). Also in older version there exists a function `create_function()`, so no need for `eval()` there, too. – KingCrunch Apr 05 '11 at 22:55
  • I'm pretty sure there was a reason why I didn't use `create_function()`. **Edit:** Yeah, checking the manual, it's due to not being able to name the function as I wanted. – Christian Apr 05 '11 at 23:04
2

Allowing free-form input of PHP code that gets executed - be it through eval() or create_function() - is simply bad design, and opens a big potential vulnerability for no good reason. It also opens the possibility of crashing a page through syntax errors.

Even the argument that the administrator can install plugins anyway doesn't hold entirely, because XSRF attacks are conceivable that manage to get malicious stuff into a text field (one request), but can't trigger a plug-in installation.

So no, I wouldn't do it; I would implement each CMS bridge as an adapter instead, and let the user choose the adapter (and if necessary enter some custom, sanitizable settings) from a pre-defined list. (Something similar was also suggested by @Wrikken in the comments)

It's your call. Chances are you will never have a problem from doing this the eval() way. And it can be argued that most of the CMSs you will be connecting with (Wordpress, Joomla) allow arbitrary execution of PHP code in the back-end anyway. But it's not good design.

Pekka
  • 442,112
  • 142
  • 972
  • 1,088
  • Ok, first of, CMSes like Wordpress already allows people to edit code from within itself. Meanwhile writing a malicious "plugin" which doesn't even install is easy considering most plugin architectures come with an "install script" concept (which gives the attacker an easy way to clean up). Though that pre-defined list of conditions idea sounds good enough - besides, users would rather select "Show debug to admin users only" than PHP-chinese-looking-stuff (no offense to Chinese people on SO). – Christian Apr 05 '11 at 22:57
  • @Christian yes, the argument can be made that all those CMSs have security holes like swiss cheese anyway. But it's not really a valid excuse to make the same mistakes IMO. I'd definitely go with a pre-filtered list that you can sanitize (as also suggested by @Wrikken in the comments). It's way more complex, but it's safe – Pekka Apr 05 '11 at 22:58
  • Actually, it's easier. Way easier. `CmsHost::cms()->is_admin();` Forget joomla or drupal and adapters; that's all to it. **:P** – Christian Apr 05 '11 at 23:00
0

Having a setting which contains PHP expression(code) that gets eval'd and it's return used as a yes/no. The best part is that the framework installed may suggest CMS-specific expressions, eg:

eval() may crash your page if any function doesn't exist or on any number of parse errors. And if bugs exist which allow user-supplied input (such as a uri requested) to even touch these evaled values, it will potentially open up your site to malicious or accidental destruction. Instead to identify the currently working framework, look for markers in the framework you're trying to bridge to, such as certain constants, functions, classes, etc. You can replace all your eval() functions with safe checks using function_exists(), defined(), etc.

bob-the-destroyer
  • 3,164
  • 2
  • 23
  • 30
  • Actually it's one of few the advantages of `eval` that it does **not** halt the script execution on parse errors. But so does `create_function`. – mario Apr 05 '11 at 22:58
  • 1
    @mario: `Eval` is very forgiving on some parse errors, but invalid functions will crash it. Per the PHP man page on `eval`: "In case of a fatal error in the evaluated code, the whole script exits." – bob-the-destroyer Apr 05 '11 at 23:01
  • I've circumvented this in the past easily, by adding a "Test" button which simulated running `eval()` (through AJAX). Since we know what to expect (as a return) we can easily "detect" errors. – Christian Apr 05 '11 at 23:02
  • @Christian Sciberras: what would happen if a user tested `unlink(__FILE__);`? – bob-the-destroyer Apr 05 '11 at 23:14
  • Oops? **:D** What if you unplugged your PC while running? – Christian Apr 05 '11 at 23:18
  • @Christian Sciberras: if you can unplug the host machine before the signal propagates from client to the host drive head as it bears down on the sector containing the file system directory entry, you _should_ be safe. – bob-the-destroyer Apr 05 '11 at 23:30
  • And what's the probability of that happening in, say, 2 seconds? 0.9 of 1? :) – Christian Apr 05 '11 at 23:33
  • Depends on client's location and service quality relative to the host and its network. It could be a window of 0.2 seconds, or it could be 0.02. Too many variables though. You'd lose time just trying to calculate your chances. – bob-the-destroyer Apr 05 '11 at 23:40