Bug #17758 Updated: magic_quotes_gpc causes more trouble than it helps

ID: 17758 Updated by: msopacua@idg.nl Reported By: php.net@odi.ch Status: Closed Bug Type: PHP options/info functions PHP Version: 4.0CVS-2002-06-14 New Comment: The fact is, that they do. It's not a false sence of security - it works for the task it's designed for. The only harm it does, is create \' in texts, when used with addslashes. That's easily spotted and takes the novice to a higher level. As for portability - if you insert data into a database within a function and you use addslashes, you __assume__ it's unescaped data. Document that, and usage of the function implies safe_stripslashes to be called on the data passed to it. No matter how you look at it, data passed to a function, always assumes some format, which should be documented. With php it implies being magic_quotes_gpc aware. With most others it means, being aware that data is unescaped. There's no real difference - just another state of awareness. As for your statement on verifying input, I thought it was implied in your initial post, under 2) - maybe I'm off and I certainly don't question your abilities, but still - you set the task of the awareness to the function, not the statement which calls it. If I use addslashes or stripslashes manually on the data passed to it - please explain how that's any different? Portable packages always need to be aware of more stuff, whether it's this configuration, or /some/long/path being in your CLASSPATH, or version x.y of jscript.dll being installed, or ... Previous Comments: ------------------------------------------------------------------------ [2002-06-14 07:49:26] php.net@odi.ch msopacua, Nothing can protect the weak and unweary. They should not write production code on their own. This feature gives them false security. The feature can be enabled and disabled per configuration. This means that you can never be sure (without looking at the ini_get) if the magic was applied to a parameter or not. Consequently you always have to check and maybe undo what the magic has done. This obviuosly affects portability (deployment). In terms of software engineering: The enabling / disabling of the magic actually *changes the contract* for the GPC, based on the setting of a config option. This simply can not be any good. ps. I never said that I wanted to avoid verifying client-sent data. As an experienced software engineer I would never make such a statement. ------------------------------------------------------------------------ [2002-06-14 06:39:16] msopacua@idg.nl I really don't get the 'poor-quality' statement. The feature protects the weak and unwary against sql injection and it's easy to work around it, using get_magic_quotes_gpc(). All you're asking for, is not having to verify client-sent data, which IMO is poor quality to begin with and link that to code-reuse and deployment problems. The problem is with your assumptions - not the feature. Example: <?php // This function should be called whenever some variable is directly inserted // into the database, when coming from $_REQUEST (and of course it's partials // $_GET, $_POST etc.). function safe_addslashes($string) { // Using a static variable, speeds up multiple calls. static $setting=-1; if($setting === -1) { $setting = get_magic_quotes_gpc(); } return ($setting) ? $string : addslashes($string); } // This function should be called whenever some variable is directly output // to the browser or a datasource that is not affected by quotes, when coming // from $_REQUEST (and of course it's partials $_GET, $_POST etc.). function safe_stripslashes($string) { static $setting = -1; if($setting === -1) { $setting = get_magic_quotes_gpc(); } return ($setting) ? stripslashes($string) : $string; } ?> ------------------------------------------------------------------------ [2002-06-14 06:33:46] hholzgra@php.net #1 most of them do not even *know* that their code relies on it, as they haven't experienced problems with quotes and stuff in queries as the magic takes care of it #2 changing magic_quotes_gpc at runtine ... requires the engine to remember which variables were filled with values from GET/POST/COOKIE this is not to difficult with the track vars or the new superglobals as there you can rely on the namespace, but it would be a nightmare in combination with register_globals=on ------------------------------------------------------------------------ [2002-06-14 06:09:14] php.net@odi.ch I forgot to mention a *quick* solution to this problem: Just make ini_set work at runtime. I know its hard because the magic happens now before execution starts. This could maybe be changed a little bit. ------------------------------------------------------------------------ [2002-06-14 06:06:42] php.net@odi.ch Agree with you that PHP can not detect if code relies on this feature. So the actual problem is: How do I tell the developer that the behaviour changed (or will change). Obviously just mentioning the fact in the release notes is not enough, because this is very subtle but quite important to know. -=| This is a problem that needs proper change management |=- So first developers and system managers must be accustomed (this is the hard part) not to use this feature. Workarounds must be provided (functions that reverse the effect). Documentation must be updated. Books must be changed (this is the timey task). Finally the specs can be changed and the feature can be removed safely. This process may take years. It's not possible to change it within two releases or so. Still, all this doesn't mean that you should simply forget about it. As I said this issue need proper change management. It takes the time it needs. But it only starts when you support it. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at http://bugs.php.net/17758 -- Edit this bug report at http://bugs.php.net/?id=17758&edit=1