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>
SantosJ - #1 - 2006-12-21 23:14 - (Reply)
Hmm, I remember reading about how the Filter adds in a extremely basic ‘taint’ type feature. It would be interesting to read more about it and how it could affect development.
wesley - #2 - 2006-12-21 23:23 - (Reply)
Is serendipity coded in PHP4? If so this is just for backwards compatability then (same execution), if it’s set at default in the .ini it needs to be restored to it’s original state, so this code seems a-ok to me.
anon - #3 - 2006-12-21 23:34 - (Reply)
what are you talking about? the code seems perfectly fine to me. how about you explain exactly why this is not good.
Pierre - #4 - 2006-12-22 00:37 - (Reply)
To anon: I’m talking about bypassing basic filtering instead of really solve the problem. * 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 I think these two points are pretty clear, no? and Wesley: filter is php5 only, this code will never be executed under php4. My point is that the only sane way is to do what everyone should have done in the first place: implement a set of functions to fetch the inputs data. That’s also why filter is easy to integrate or to be used directly.
wesley - #5 - 2006-12-22 11:56 - (Reply)
Yes this code is PHP5 only, my point exactly, so to get the same input as you would get in PHP4, this automatic filtering has to be undone.
Pierre - #6 - 2006-12-22 13:11 - (Reply)
"Yes this code is PHP5 only, my point exactly, so to get the same input as you would get in PHP4, this automatic filtering has to be undone." Then no, this is exactly what you should not do but do it the safe way (and portable), implement your own functions as fall back if you still did not have them in your current projects. Doing so will let you use any configurations without having to worry about ext/filter presence or absence.
wesley - #7 - 2006-12-22 14:25 - (Reply)
Why should my own functions be fallback if they do exactly the same thing as ext/filter? Why not ditch ext/filter instead? Besides, the function is not yet ready, it was just thrown into php 5.2 for whatever reasons. And it is seriously limited.
Pierre - #8 - 2006-12-22 14:37 - (Reply)
Let me clarify my point. HAVING already your own filtering functions is the way to go. Then you can easily integrate them with ext/filter if required/wished, transparently. All you will have to update are these functions (that’s true for each changes related to input filtering, instead of digging all your codes)...
Stefan Esser - #9 - 2006-12-22 14:43 - (Reply)
Pierre, please educate yourself about Suhosin. Suhosin is not a tool to help secure application development but a tool to protect servers from holes in applications that only exist because of insecure design decisions of PHP. Before you rant against a tool you should first learn what it is. Therefore all your points (as usual) are bullshit BTW: Good to see that you have no valid technical statement that renders any of my points wrong. If you rely on ext/filter for your input filtering then you have no security on servers that don’t have it. If you have your own input filtering functions, then why use ext/filter? It is only a performance and ressource killer. Ohh btw could you explain what your answer to "ext/filter is just a new irritating API to already existing functionality" has anything todo with the API provided by ext/filter? Should I remind you about the meaning of API? Yeah well, what should I expect from someone who introduces bufferoverflows in a space trimming function.
JW - #10 - 2006-12-22 14:50 - (Reply)
"There is no way to provide input filtering in PHP while preserving the original data." Replace the superglobals class MyGet implements ArrayAccess { protected $raw; function __construct(array $raw) { $this->raw = $raw; } .... } $_GET = new MyGet($_GET); PS. Dynamically created & executed PHP? Notice: Undefined index: preview in /tmp/dctpl_effdb8d4f58f8da314277a0178d471ee.php on line 144
Pierre - #11 - 2006-12-22 14:57 - (Reply)
"$_GET = new MyGet($_GET);" and where do you store the original _GET? and you cannot force any rule using only user land code. "PS. Dynamically created & executed PHP? Notice: Undefined index: preview in /tmp/dctpl_effdb8d4f58f8da314277a0178d471ee.php on line 144" Thanks for the notice, Dotclear2 thingies (cache related), I have to take some time to fix this issue, but I’m looking for troubles, I’m running it under 5.2-dev ![]()
Pierre - #12 - 2006-12-22 15:05 - (Reply)
"Suhosin is not a tool to help secure application development but a tool to protect servers from holes in applications that only exist because of insecure design decisions of PHP." My point if you read it correctly is about writing bugfree softwares. About the insecure design decisions, I remember you that you were part of the security team, you are (or I should say were as you are clearly doing everything you can to discredit any single php developer, Zend coworkers or anyone having different opinions than your highness) a PHP Core developer with ALL necessary karma to fix any kind of issues, so please stop this little game. "Therefore all your points (as usual) are bullshit" Your attitude in general is a shame, regardless of your knowledge or expertise. You have no way to communicate without insults and personal attacks. I’m sorry for you. Unlike you, I never claim to know everything or to write bug free codes. For this exact reason I mailed numerous times internals to call for review the filter code base, where were you? Besides refusing to report bugs for some stupid reasons like I will reject it because of the susoshin patch (point me to one bug I reject because of this patch).
Stefan Esser - #13 - 2006-12-22 16:32 - (Reply)
Pierre stop comparing apples with pears. There is a big difference between bugs in Suhosin and ext/filter. In ext/filter applications rely on it working 100% in all cases, because otherwise the security of all these applications is doomed. In Suhosin a bug means that some protection that is an addon does not work. Applications don’t get insecure because one part is not working. (Unless there is a code execution vulnerability in it). The security of applications is not compromised if one of Suhosin’s protection does not work. The only thing that happens is that one additional protection is broken. I don’t claim to write bugfree code. And my attitude is not a shame it is just a reaction on your arrogant behaviour. And btw you make me laugh. I announce the creation of an input filtering library because I consider ext/filter useless and you try to make it look like I only consider ext/filter useless, because I write my own library. Get your facts straight. You live in a fantasy world and it is time for you to wake up.
Stefan Esser - #14 - 2006-12-22 16:39 - (Reply)
Ohh btw… You should know that I don’t do marketing. You propaganda about self marketing/promotion is really amusing. Maybe you realise that unlike Zend or Shiflett I have not given interviews about me leaving the team, although I had tons of emails asking about interviews. Marketing has no place in security.
Pierre - #15 - 2006-12-22 16:51 - (Reply)
When someone considers that everyone outside his immediate sphere should grow up, get a clue and learn the facts or simply stop saying BS, it is time for him to ask himself. "Marketing has no place in security."Just a matter of definition. What you are doing is bad marketing, trying to discredit the work of many people by constantly saying that they are clueless or always made the wrong choice (let forget your today’ comment about me). You are lying to yourself and to us if you still think that you are not doing any marketing or self promotions. Your propaganda trips is getting old, your complete lack of social ability is now your only excuse. It is time for you to learn the basic notions of respects, whether you agree or not with your interlocutor is irrelevant.Thanks for this discussion, but I stop here, feel free to comment further if you have something (if possible interesting) more to say, no moderation here.
Cristian Rodriguez - #16 - 2006-12-22 18:07 - (Reply)
How you pretend Flyspray to be compatible in the case the system adminsitrator has changed the "default filter" eh ? care to elaborate what "problems can this cause ? " and provide a bettee solution that does not need gazillions of new code to mantain ? and no, we are unlikely to use, ext/filter, first, ‘cause Flyspray needs to be compatible with ext/filter-less php4 and as I already discussed this with you on IRC, the api is a real pain in the ass, plagued with tons of constants, un-intuitive behaviuor and the main problem,. no built-in object oriented interface.
Pierre - #17 - 2006-12-22 18:11 - (Reply)
"How you pretend Flyspray to be compatible in the case the system adminsitrator has changed the "default filter" eh ?" I did not pretend that. I say that the filter extension is 100% BC when the default filter is left to the default value (which means no filtering at all).What I’m saying is that this code is not a good solution. It is better and easier to centralize the input filtering/fetching functions and only use these functions in your application (on that point, Stephan and I agree, at least).
Cristian Rodriguez - #18 - 2006-12-22 18:38 - (Reply)
>What I’m saying is that this code is not a good solution. It is better and easier >to centralize the input filtering/fetching functions and only use these functions >in your application (on that point, Stephan and I agree, at least). Well…what is the best solution then ? ahh.. and there is such centralized place in flyspray, but we need to get around possible misconfigured places were, f.e default filter is configured to ! unsafe_raw which will break the app badly. Finally, is my best wish that this new stuff in php will succeed ( and you know that, I have spent significant amount of time this year, testing new PHP features, and submitted and important amount of bug reports..), however, I sincerly don’t see ext/filter massively used due to it’s api , the lack of oop interface, and the complexity that requires to be added in the case we want backward compatible code.
Pierre - #19 - 2006-12-22 18:51 - (Reply)
"Finally, is my best wish that this new stuff in php will succeed ( and you know that, I have spent significant amount of time this year, testing new PHP features, and submitted and important amount of bug reports..), however, I sincerly don’t see ext/filter massively used due to it’s api , the lack of oop interface, and the complexity that requires to be added in the case we want backward compatible code." Yes, I know it and I follow flyspray development (from a feature and stability point of view only) closely. Despite Stephan’s comment, this post is not an attack against S9Y or Flyspray but a (strong) recommendation to do not solve this issue using such horrible solutions.The lack of OOP is not a problem, the API is small and easy to use. It is a joke to integrate it in any existing code. If you already have input filtering functions, I do not see any difficulty to integrate ext/filter. I may help in this area, or let me know what is missing (besides OO:).
Cristian Rodriguez - #20 - 2006-12-22 21:16 - (Reply)
>You can build filter as a shared module. No, it won’t build shared, —enable-filter=shared makes no effect at all. there is even a comment in config.m4 "dnl This extension can not be build as shared when in PHP core" >a) apply an unofficial patch to php and unofficial patch that has been applied to all BSDs, OpenSUSE, DotDeb packages, Mandriva, Ubuntu feisty(afaik) and other countless OS, so the admin just need to install any of them ![]()
Pierre - #21 - 2006-12-22 23:27 - (Reply)
"No, it won’t build shared, —enable-filter=shared makes no effect at all. there is even a comment in config.m4" You can build it as shared module. You can’t do it when you build it inside php src tree. Little but important nuance
"and unofficial patch that has been applied to all BSDs, OpenSUSE, DotDeb packages, Mandriva, Ubuntu feisty(afaik) and other countless OS, so the admin just need to install any of them" I stopped to use most of those binaries due to countless troubles. By the way, filter is also available and updated in all these repos, just as easy to update
. However that does not make this patch more official and has nothing to do with his main argument: having to recompile. By the way, you did not answer my last questions or help proposition. I hope you will (re)consider filter, especially as flyspray is still in development (well advanced/beta but still
. It would be nice to support it from the first stable release.
Cristian Rodriguez - #22 - 2006-12-23 00:01 - (Reply)
I wrote a reponse here http://blog.flyspray.org/archives/1…
Pierre - #23 - 2006-12-23 01:45 - (Reply)
replied there too. But you seem to think that I was attacking flyspray, its quality or developers, that’s sad. Please forget what Stephan is trying to spread. I never intend to attack any project or developers but to point out what I consider as a bad solution. anyway, time for season’ parties ![]()
Cristian Rodriguez - #24 - 2006-12-23 01:49 - (Reply)
No , I never thinked that you are attacking us but constructive critisism is always welcome.
Pierre - #25 - 2006-12-23 02:10 - (Reply)
Oh My… One day I will understand what can be considered as constructive, or should I have to write the alternatives for all projects using this snippet? Anyway, we made our points, let see what come out of them ![]()


