php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #17758 magic_quotes_gpc causes more trouble than it helps
Submitted: 2002-06-14 03:23 UTC Modified: 2002-06-14 07:49 UTC
From: php dot net at odi dot ch Assigned:
Status: Closed Package: PHP options/info functions
PHP Version: 4.0CVS-2002-06-14 OS:
Private report: No CVE-ID: None
 [2002-06-14 03:23 UTC] php dot net at odi dot ch
magic_quotes_gpc  is a bad idea and should be abandoned for the following reasons:

While the option may look very tempting at the first glance there are some caveats however:

  1. Most parameters do not go to a database.
In a web application most form field are used internally without the need to store them in a database. Magic quotes  cause troubles in these cases.
Moreover the data passed to the application is not the data entered by the user if it was processed by magic quotes. This is undesireable.

  2. Impedes code reuse.
If you feed data from either form parameters or internal data sources into the same function then your function must know if the data was processed by magic quotes or not.

  3. Bad surprises at deployment time and code portability.
If you do not carefully check if this parameter is set on your development and production system you can run into troubles. Especially if you can not change the settings on one system (because the hoster does not let you).

  4. Behaviour can not be controlled at script runtime.
The ini_set does not help in this case. Even though the parameter can be modified at runtime the behaviour does not change. Consequently you are bound to the php.ini settings (which may be not under the developer's control).

I therefore request that this (and related) option be removed from future versions of PHP and the default behaviour should be FALSE.

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2002-06-14 03:33 UTC] mfischer@php.net
Abandoning/Deprecating it is not an option -> the BC impact is too nig.

If you take a closer look in the PHP4 distribution you will find a file called php.ini-recommended , magic_quotes_gpc are turned off there.
 [2002-06-14 03:33 UTC] mfischer@php.net
"too big" I meant
 [2002-06-14 03:44 UTC] php dot net at odi dot ch
Recommendations do not help against the unwise and careless.

I think deprecation actually is an acceptable option here. The Java API has been using this mechanism from the start on successfully. The developer is given one release time to change his code.

The issue should at least be discussed somewhere and not just be closed by an individual. You can't just say "this is design flaw, so we do not change it".
 [2002-06-14 03:46 UTC] derick@php.net
Too much ppl rely on this, this cannot, I repeat CANNOT, be removed now. I agree it's a silly thing, but breaking scripts for a lot of users is not an option.

Derick
 [2002-06-14 03:57 UTC] php dot net at odi dot ch
I was never talking about "now". I said "in the future".

I do not want to urge anybody. Nobody is forced to use a new PHP release with existing code. Nor can anybody expect to use a new PHP release without touching existing code.

Once more may I ask you to reopen this request. Otherwise you will forget it. We want PHP to be of high quality. This includes we have to get rid of poor-quality concepts.
 [2002-06-14 05:43 UTC] hholzgra@php.net
the java way of deprecating things does not work
here because of the magic used, there is no way
(i can think of) how PHP can detect that code 
relies on this feature at any point, this is
*very* different to just deprecating a method,
member variable, interface, class or package

so turning it of by default in php.ini-recommended
is all we can do about it for now, turning it off
by default would be a bad idea as the effect would
be far less obvious as the problems that arise with
register_globals=off, and that move already caused
us a lot of trouble, and totaly removing the 'feature' would be even worse


 [2002-06-14 06:06 UTC] php dot net at odi dot 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.
 [2002-06-14 06:09 UTC] php dot net at odi dot 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:33 UTC] 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:39 UTC] msopacua at idg dot 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 07:49 UTC] php dot net at odi dot 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 09:18 UTC] msopacua at idg dot nl
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 ...
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Sat Apr 27 16:01:29 2024 UTC