how to do not work around filter (don't be lazy :)
Posted by Pierre in
Uncategorized
Friday, December 22. 2006
(Update on 2006/12/22 click to read more)
While reading the very long "taint mode", I had more and more the feelings that many developers ignore ext/filter or simply work around it. At the same time many argue about the necessity of this taint mode. It is a self contradiction and makes me wonder about the real reasons, I will post later about the existing taint mode available since php 5.2.0.
On the other hand, the same persons worked around ext/filter with ugly hacks. Edin pointed me to one of these horrible code sin Serendipity, as I saw this code in other applications like flyspray, I think it is time to raise your attention about what to do not do (The enlightenment has been brought to the author of this code, which is the same in flyspray and s9y
. Here is what you should NEVER do:
<br /> if (extension_loaded(‘filter’) && filter_id(ini_get(‘filter.default’)) !== FILTER_UNSAFE_RAW) {</p> if(count($_GET)) { foreach ($_GET as $key => $value) { $_GET[$key] = filter_input(INPUT_GET, $key, FILTER_UNSAFE_RAW); } } if(count($_POST)) { foreach ($_POST as $key => $value) { $_POST[$key] = filter_input(INPUT_POST, $key, FILTER_UNSAFE_RAW); } } if(count($_COOKIE)) { foreach ($_COOKIE as $key => $value) { $_COOKIE[$key] = filter_input(INPUT_COOKIE, $key, FILTER_UNSAFE_RAW); } } if(isset($_SESSION) && is_array($_SESSION) && count($_SESSION)) { foreach ($_SESSION as $key => $value) { $_SESSION[$key] = filter_input(INPUT_SESSION, $key, FILTER_UNSAFE_RAW); } } <p>}<br />
It is not only plain stupid (to be polite) but laziness is the first cause of most security flaws. Do it the right way:
- Implement your input functions as fall back when filter is not available or use filter in them
- Never use directly the super globals outside these functions
But to do it like in the code above is calling for troubles.
Update
First, I like to say that this post is not an attack against the Serendipity or flyspray developers, I hope they don’t feel offended . My comments and suggestions are related to this exact portion of code. Now that I clear the misunderstanding with Stefan. Let start to answer.
As a christmas wonder, we do agree on one point, each project should have its own input functions. This was the main point of my post. This solution has the advantage to centralize the operation and to force the developers to use these functions (coding standard or whatever you use to define rules within a project). It will also protect you from your own laziness/distraction ![]()
Stefan was saying that everything in filter can be implemented in userland, that’s plain wrong. There is no way to provide input filtering in PHP while preserving the original data. As soon as you replace the contents of a superglobals, the original contents is not available anymore (if you rely only on GPC super globals). And this is one of the good thing in filter, the ability to fetch the same input using different filters when and where you need it. Using your own input filtering can provide similar features but with no waranty that one will use them or will not fetch the original values before the initialization of your input filters.
"ext/filter is just a new irritating API to already existing functionality"
I suppose Stefan was talking about the default filter, which is set to "unsafe_raw" by default. "unsafe_raw" does not filter the inputs, they are left intact, 100% backward compatible. Yes, if a host uses a default filter, this change is certainly justified or requested, does that mean you are out of luck and ext/filter is plain crap? no. As I said in my post, if you have your own input functions, it is a piece a cake to provide full support of the new ext/filter within your applications or modules. Do I really have to remind that havining GPC super globals all over the code is not a good practice?
"that potentially introduces new security holes, because it reimplements existing stuff again"
Aber sicher! Come back, every single line of code potentially introduces security issues or bugs. That’s true for everything, even for S#?*in patch. Is it an argument to do not provide ext/filter? Be serious half a second.
"ext/filter is a similar stupid idea like magic_quotes_gpc. When you use it, the security of your application depends on the server configuration and no longer on your application code."
That’s another (double) wrong statement. As said earlier, there is no default filter by default. That does not prevent you to use ext/filter API to fetch what you need. It is part of your development rules, as you have to think about what you are doing instead of spreading the gpc all over your apps. When a default filter is used, using the filter functions will give what you need and when you need it, regardless of which filter is used by default.
I do consider the super globals (_POST/_GET/_COOKIE/...) as a mistake, their usages should be restricted to a very small area of an application. ext/filter is a step to the right direction (it is not the panacee, nothing can solve all needs or problems).
And about the server configuration, no matter what you do or say, the security of your applications depends (partially or totally) on it.
"Supporting ext/filter makes your code more complex"
For what I heard and given my own experiences, it makes the code less complex, easier to maintain and much more clear (all inputs management code is centralized, policy can be enforced, a script arguments are clearly identified, ...). You do not have to worry about wide GPC usages all over your applications (as developer lead or when you audit your apps, it is an important factor). And yes, the centralization of the input filtering can be done without ext/filter. ext/filter provides a way to force it.
"and in case of a bug in ext/filter you have no chance to protect your users, because security only relies on the admin’s will to upgrade."
You can build filter as a shared module. By the way, if I use the S#?*in patch to secure my application/php, am I not in the exact situation? My security only relies on the admin’s willing to a) apply an unofficial patch to php and b) keep it uptodate by patching and recompiling? or maybe you have no bug.
"Additionally ext/filter will keep a copy of all GPC variables in memory"
I’m fixing this issue, by providing a true JustInTime for the auto global (and unicode support).
Update #2
PHP codebase must be pure crap from a security point of view, that’s the commercial and marketing main argument of the Susoshin patch. Whether it is true or not, I let you make your own judgments (Zeev’s reply to the /. misquote is a good place to start).
But now one thing is sure, all attacks and recent moves have one and only goal, self marketing and promotions. How can it profitable if there is "virtually" nothing wrong in PHP code or features? The good thing is that this position is now clarified and my last hopes about their real intention have been killed (no big deal). Let enjoy 2007, I was slowly getting bored lately, 2007 sounds promising
</Dallas>



