Should have a more convenient approach to blacklisted functions

[narfbg]

When a function is blacklisted, the only way to properly detect that is by handling the output of ini_get('suhosin.executor.func.blacklist'). This is a pain to do when you need multiple checks and makes function_exists() and is_callable() practically useless when you want to determine if e.g. exec() can be called safely.

Also, terminating script execution when a blacklisted function is executed isn't the most ... friendly approach. Returning FALSE, NULL, etc. would be way more convenient.

I'm no C coder and I'm not familiar with the PHP internals, so I don't know if those two behaviors are even possible, but to better understand why they are important I'll explain how it got my attention.

I'm a CodeIgniter Reactor engineer and for better detection of MIME types on file uploads, we utilize dangerous functions like exec(), popen() and system() to call /usr/bin/file, when they are available. Now, I see that the Suhosin extension has a way of handling this stuff, but with CodeIgniter being a framework and largely used in shared-hosting environments, in most cases it's not even possible to have somebody configure _suhosin.upload.verificationscript. And even if so - the function blacklist still makes it almost impossible to work out of the box.

I know this is all configurable, but I still believe that it would be way better if at least one of the two feature requests above could be satisfied. Should even make Suhosin more popular and easier to adopt.

[sjorek]

:-1:

Your wishes weaken the security of suhosin. If your project relies on such functions than improve it's installation procedure to check if the requirements can be fulfilled. If not stop supporting these environments.

My recommendations:

• rewrite your detection of MIME types in pure PHP, possibly using http://php.net/manual/de/book.fileinfo.php
• ~~if you can not rely on this module or want more details than just mime-types, implement the corresponding fileinfo-module or necessary functionality in pure PHP~~
• learn how to code in C and improve PHP's file info capabilities

See next comment …

[sjorek]

An update to my previous comment - I've seen you're already using the first recommended approach utilizing PHP's fileinfo-extension. As as source of inspiration you could enhance your implementation with a pure PHP solution like this:

• https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L1077 and https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L540

Hope you're not offended by my down vote. :see_no_evil: :hear_no_evil: :speak_no_evil:

[narfbg]

@sjorek I understand that making blacklisted functions return FALSE/NULL is probably not a great idea (even for reasons beyond security) and may not be an easy/enjoyable task. In fact, now that I think of this again this was probably a stupid idea. However, that's not an excuse to not do anything about this issue and I certainly don't see how that weakens security if the functions are still not executed (after all, I'm not proposing to get rid of the blacklist functionality, only to improve it). Could you elaborate on this?

What I would now suggest is simply patching function_exist() in order to recognize the suhosin.executor.func.blacklist option. It already does that with the original disable_functions ini setting, so how hard could that really be? I'd much rather tackle that with my limited C skills than trying to understand Fileinfo - something not only related to PHP's internals but the file system as well.

There's also the problem of PHP typically being restricted in many ways, which I assume is one of the reasons for Fileinfo not being reliable enough already.

I don't feel offended by your down vote (although I didn't know it was a case of voting) and your link is indeed quite interesting, if not anything else - effectively maintaining a database of MIME type patterns would be a framework in it's own right. I probably would end up doing that, because I like dealing with non-standard stuff, but this won't solve the problem for the millions of other PHP apps and their developers.

What's ironic in my case is that using exec() for MIME detection actually helps me to maintain a level of security, but that may not be the case for somebody else - call a script, start a daemon, grep some logs, you name it ... everything that goes through the command line gets twice as complicated.

It seems to me like you're talking more from a sysadmin's point of view and in many cases I've been very restrictive myself, but when it comes to PHP programming, I assure you - every single developer would love to just have the convenience of function_exists() returning FALSE on blacklisted functions and they would all hate it for not having this.

[NewEraCracker]

As far as I know after this commit, when calling function_exists() with a blacklisted function name, it does return false.

https://github.com/stefanesser/suhosin/commit/6b00f2346c2529a9169cf1c04580e293a77f6a71

Please let me know if that is not the case.

[narfbg]

@NewEraCracker Well I'm glad somebody else thought about this as well. :)

I'll have to test that again, but I'm pretty sure it didn't work at the time I started this thread. I do admit that I didn't get the patch from github back then, so it could've been due to me using older code.

[stefanesser]

Suhosin did always have code that was supposed to return FALSE in function_exists() for blacklisted functions. The code just contained a problem that made this code fail most of the time. This problem has been fixed in the above mentioned commit.

[sjorek]

Ok - I always thought that even testing a blacklisted function is a security violation, as it's a hint that someone tries something not being allowed. Obviously I was wrong, referring to https://github.com/stefanesser/suhosin/issues/18#issuecomment-24064909 and https://github.com/stefanesser/suhosin/issues/18#issuecomment-24065609

---

Nevertheless I'd like to give a reply to https://github.com/stefanesser/suhosin/issues/18#issuecomment-24058806 with my thoughts ( written when I was not yet aware of the above hint ):

However, that's not an excuse to not do anything about this issue and I certainly don't see how that weakens security if the functions are still not executed (after all, I'm not proposing to get rid of the blacklist functionality, only to improve it). Could you elaborate on this?

Your suggestion means that PHP shall continue executing a script even if a security-violation occurred. Well, exploits, which already got that far, can continue testing if there are other functions to abuse. Obviously not the best idea.

There's also the problem of PHP typically being restricted in many ways, which I assume is one of the reasons for Fileinfo not being reliable enough already.

Yep, your assumption is right.

Example - try it for yourself on - works on most Linux, BSD (incl. Mac OS X) having file and/or PHP with fileinfo-module installed:

# create a GIF which obviously isn't a GIF
$ echo 'GIF89A' >not-a-gif.gif

# detect the file's content-type
$ file not-a-gif.gif 
not-a-gif.gif: GIF image data

# detect the file's mime-type
$ file --mime not-a-gif.gif
not-a-gif.gif: image/gif; charset=us-ascii

# use PHP's fileinfo-extension to detect the file's mime-type
$ php -r "\$finfo = finfo_open(FILEINFO_MIME_TYPE); echo finfo_file(\$finfo, 'not-a-gif.gif'); finfo_close(\$finfo);"
image/gif

Hint: PHP's fileinfo module simply replicates the file-command-line tool (utilizing a patched version of libmagic ) and therefore needs it's “mime.magic”-database.

It seems to me like you're talking more from a sysadmin's point of view and in many cases I've been very restrictive myself, …

Nope - I just have a strong emphasize on quality and durability and code 99% of my time. :smirk:

… but when it comes to PHP programming, I assure you - every single developer would love to just have the convenience of function_exists() returning FALSE on blacklisted functions and they would all hate it for not having this.

I wish more PHP-Programmers were more cautious about what they do. The cheapest solution is not always the best (and often even not the most efficient).

---

How about the following approaches (for CodeIgniter):

• (a) : Encapsulate function_exists in a new function suhosin_function_exists which in turn checks suhosin's blacklist and eval-protection configuration first before actually executing function_exists
• (b) : Refactor your code to test the hosting environment's capabilities during installation-time and not during runtime and safe the detected configuration so you can choose the right approach at runtime. This could result in faster execution, smaller memory footprint and most important, it will result in a much more user-friendly solution, as I'd like to know if there is something missing before and not when I need it …
• (c) : As already mentioned above, enhance/replace your solution with something like (I don't judge the code's quality here, I just refer to the idea of how to implement it):
  • https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L1077
  • https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L540
• (d) : Reimplement the PHP's fileinfo module in pure PHP. It's not that hard - it works quite similar to solution (c).
• (e) : Combine (a) with (b), (c) and/or (d)

Solutions (c) and (d) don't force you to maintain a own mime-type database, as you could rely on (=convert) file's mime-info database and use this. It's usually located in /usr/share/file/magic/ or the like. RTFM man file. :innocent:

So if you're interested in solution (a) or (d) I'd suggest you create a separate (composer-based) project or for solution (c) you could create a branch at CodeIgniters-Repository. Ping me back if you have something I can assist.

Cheerio, Stephan

[narfbg]

@stefanesser @NewEraCracker I now see why it didn't work 10 months ago and why it wouldn't work now unless I download from GitHub - the last version available (and listed in the changelog) on the website is 0.9.33, while the above mentioned commit is (to be) released as 0.9.34. I'll take your word on it being fixed for I don't want to deal with compiling PHP right now. :) Thanks a bunch!

@sjorek I'll take the issue-closing-comment opportunity to reply to you as well:

However, that's not an excuse to not do anything about this issue and I certainly don't see how that weakens security if the functions are still not executed (after all, I'm not proposing to get rid of the blacklist functionality, only to improve it). Could you elaborate on this?

Your suggestion means that PHP shall continue executing a script even if a security-violation occurred. Well, exploits, which already got that far, can continue testing if there are other functions to abuse. Obviously not the best idea.

I already admitted that modifying i.e. exec() to force return FALSE when blacklisted was a stupid idea, but I somehow felt I wouldn't agree with your arguments about it ... seems like I was right about that. :)

PHP is being bashed for a lot of it's problems, some more obvious than others and my initial suggestion here would be one of the latter, but here starts our disagreement - it's not about security, but simply because it's not the right way to do it. Raising an exception would've been a way more proper approach.

This is why I said the sysadmin-point-of-view thing ... your comment makes it clear that you're (almost) looking at exec() like an exploit by itself - that's exactly how a system administrator and/or a security expert would look at it by default in order to ensure that no threat exist. But exec() is not an exploit, nor is system(), etc. It's a dangerous functionality to give in everybody's hands and surely blocking it in every shared hosting environment is justified, but solely executing exec() doesn't qualify as a violation.

To give you an analog ... I'm sure all three of you have been on IRC and you most likely know that most networks would ban all open proxies by default. Why is that? It's not like it's any different than spoofing your original host name to protect yourself from the script kiddies. It's because the kiddies will abuse the same proxies to flood the network. This doesn't mean that you should kline a regular user who's proxy somehow slipped around the OPM bots.

I wish more PHP-Programmers were more cautious about what they do. The cheapest solution is not always the best (and often even not the most efficient).

Couldn't agree more here. :)

How about the following approaches (for CodeIgniter):

• (a) : Encapsulate function_exists in a new function suhosin_function_exists which in turn checks suhosin's blacklist and eval-protection configuration first before actually executing function_exists

Obviously, even after your second comment, you haven't noticed that I'm not doing function_exists(), but function_usable() checks in there. :) I had already worked my way around this in CI, I posted here because I felt it wasn't quite right to do such expensive checks in the first place and because there would be many more people suffering from this problem that wouldn't even know what's going on.

• (b) : Refactor your code to test the hosting environment's capabilities during installation-time and not during runtime and safe the detected configuration so you can choose the right approach at runtime. This could result in faster execution, smaller memory footprint and most important, it will result in a much more user-friendly solution, as I'd like to know if there is something missing before and not when I need it …

There is no installation time. IMO, an installer would make sense in a CMS, a message board or simply in an application written in a language that needs to be explicitly compiled, but not in a framework. It's the framework that's used to write an installer.

• (c) : As already mentioned above, enhance/replace your solution with something like:
  • https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L1077
  • https://github.com/JamesHeinrich/getID3/blob/master/getid3/getid3.php#L540
• (d) : Reimplement the PHP's fileinfo module in pure PHP. It's not that hard - it works quite similar to solution (c).
• (e) : Combine (a) with (b), (c) and/or (d)

Solutions (c) and (d) don't force you to maintain a own mime-type database, as you could rely on (=convert) file's mime-info database and use this. It's usually located in /usr/share/file/magic/ or the like. RTFM man file. :innocent:

I will probably do that indeed, thanks. However, this is where I'd probably have problems with how PHP is being limited in many ways, but even to greater extent than the original Fileinfo extension may be affected - I may just not have access to that MIME-type database. In fact, with the amount of developers reporting incorrect results, I'm probably better off maintaining my own database anyway. Chances are that the one available (if there is one) on the system is missing quite a lot of entries. Not to mention Windows support. :)

---

Thanks again to all three of you, even if only for the discussion itself - it was worth waiting a few months for a reply. :)

Cheers!

[sjorek]

Ack, sorry for not researching thoroughly enough. :metal: