In complement to @tom answer.

Magic Quotes

Automatically parsing the whole entries and adding magic quotes is both creating bugs and useless.

• Useless as you cannot rely on magic quotes to secure your input (multy-bytes encoding bugs for SQL injections is an example). So you need to apply a real filter before saving your data to a database
• Creating bugs: If you need to really escape your data before a save in database you have to check that it’s not already escaped (and the simple fact this settings exists and may be enforced by the hosting environment makes that you have to check this setting was set or not).
• Creating bugs: All the data sent by the user is not always dedicated to a database storage. Escaping it may break the content, think about a json content for example, or even file content with the dangerous magic_quote_runtime
• Creating bugs: All database storage are not escaping quotes the same way…

So Why?, why do we see such function in a CMS?

• see that here it’s an add_magic_quotes function, that can be used on a dedicated array, maybe not on _GET or _POST. But effectively the fact this function is just using addslashes and not a database dedicated function makes it quite bad.
• The fact the hosting provider may enforce an automatic magic quotes is a nightmare for a CMS developper. Either you detect it and tell the user you refuse to run, or you have to manage the fact the content may or may have not be magically-addslahed… and to put everyone in the same state, you run the non-addslashed content in this function so that at least everyone is in the same (bad) state.
• From what I can see on Wordpress, before the save a stripslahes_deep is performed in the wp_insert_post. And add_magic_quotes is usually performed on data pulled from Db before this data is send to the wp_insert_post. This may me think the problem is effectively to add slashes before removing them… maybe because sanitize filters which append before the save expect content with slashes, or maybe because no one remember why the code is running in this way 🙂

register_globals

Seems that this is the way to implement a Registry pattern in wordpress… They wanted to make the code simple to understand, and to allow a simple way to access importants objects like the query or the post. And an object oriented Registry class was not in the simple PHP way, where the $_GLOBALS array is already an existing registry.

Having a Registry is a perfectly valid thing in an application. a register_global thing is dangerous only if you allow some user input to override your valid secure input. And of course only if this secure input is taken from $_GLOBALS elsewhere (or with global keyword).

The dangerous part in the function here is the part of the function you have extracted, the loop on $query->query_vars. You will have to track the calls to see if user injected keys could run throught wp_parse_args and finish in that function. But the next part of this function is fixing $_GLOBALS content for several objects:

$GLOBALS['query_string'] = $this->query_string;
$GLOBALS['posts'] = & $wp_query->posts;
$GLOBALS['post'] = (isset($wp_query->post)) ? $wp_query->post : null;
$GLOBALS['request'] = $wp_query->request;

So at least theses globals cannot be overwritten by user input and are safe.

So, theses functions are bad. But you can use them if you understand what they do and what you need to do to prevent the bad effects. And when you want to implements a simple framework for developpers, available on a very wide environment you sometimes have to use them.

But for sure it’s a bad practice, you can certainly find bad wordpress plugins using *$_GLOBALS* in the wrong way or misusing the add_magic_quotes to data pulled from db wordpress concept. But there will be years before a Zend Framework CMS gained such a big number of contributions.

NOTE: This answer was originally posted at StackOverflow.com by regilero

Wordpress. I’ve spent a lot of sleepless nights trying to answer the only one question: “Why??”

Since I’ve faced with its source code I hate it. It is awful. And let my post(and reputation as well) will be minused but it’s true.

It doesn’t have core. There is a rubbish of code instead of core. It reminds php3. Huge heap of unrelated and unlogical functions are used in it. “Copy and Paste” – the only one design pattern is used in wordpress.

Yes, the have emulated using of prepared statements. But why they don’t use PDO, or mysqli? They have copypasted almost all PDO functions but haven’t use it instead. Using of mysqli instead of mysql requires even less efforts.

They use myql_real_escape_string. But there is still something like protect_string_strongly, protect_string_weakly. There is no only only one function – do_not_protect_string_i_believe_my_users.

Global variables – is the philosophy of wordpress. “If we don’t know how to change this var we’ll mark it as global var and everybody will be happy.” – here is what wordpress developers thought when they developed hellpress.

Every new version contains a lot of new in design, they add new default themes, they change background color in admin area from #ccc to #cdcdcd, they use dropdown menu in admin area instead of accordeon. And it awesome. But they do not improve its code.

Did you read comments in WP “core”? No? And I did. They are “awesome”. Something like “What this function is called for? Let’s it leave just in case.” or “Do not hardcode this in new version.” and so on.

The only one answer to the question “Why?” I got is: “Because it works. And if it does work do not touch it!!!”

wordpress.org is one most visited sites in the world. Why? Because nobody able to understand wordpress’s logic. Everybody every time needs to ask something on the forum or read in the codex.

I hope you understand my point of view.

NOTE: This answer was originally posted at StackOverflow.com by Ruslan Polutsygan

One of the main differences between mysql_real_escape_string() and addslashes is that mysql_real_escape_string() works in conjunction with the character set so it knows how to properly escape data based on the character set.

Generally I think the best approach is to use a request class and do all your stuff there.
That way there is only one place where you deal with GET,POST,COOKIE, SERVER etc which makes it far easier to manage in contrast to having a bunch of random functions doing different things. That’s just a recipe for disaster.

NOTE: This answer was originally posted at StackOverflow.com by gus

Magic Quotes

The following text is taken from PHP.net

http://www.php.net/manual/en/security.magicquotes.why.php

There is no reason to use magic quotes because they are no longer a supported part of PHP. However, they did exist and did help a few beginners blissfully and unknowingly write better (more secure) code. But, when dealing with code that relies upon this behavior it’s better to update the code instead of turning magic quotes on. So why did this feature exist? Simple, to help prevent SQL Injection. Today developers are better aware of security and end up using database specific escaping mechanisms and/or prepared statements instead of relying upon features like magical quotes.

addslashes() vs mysql_real_escape_string()

The reason why you should use mysql_real_escape_string() is because it’s a “MySQL function” and is created especially for escaping user input before it’s executed in a mysql query, while addslashes() is a “PHP function”. That probably sounded a little weird, but there’s one important difference between the two and it has to do with the use of single- and multi-byte characters. You can still inject databases protected by the addslashes function, but injecting databases protected by mysql_real_escape_string is far more difficult. You can read more about it HERE

Register Globals

The reason why you should NOT use register_globals is because variables become accessible to everyone, which means that in the following example you would be able to set $access to true if it hasn’t been initialized before

<?php

if (isAuthenticated()) { $access = true; }

if ($access == true) {
  include(controlpanel.php);
}

?>

The above code would give you sh#! loads of problems, but if we initialize the variable first by adding the following to the top of the page

$access = false;

…we should be fine even if we have register_globals ON

So, if the Wordpress team have initialized all variables (which they probably have) then you don’t have to worry about the use of globals.

Conclusion

It’s definitely bad practice using any of those 3 functions/features and I would never do it myself. Are you sure you’re working with the latest version of Wordpress? Like someone commented, if you are using the latest version it’s because of laziness or worse it’s still in there. I’ld never use Wordpress for anything other than blogs that doesn’t require much security..

NOTE: This answer was originally posted at StackOverflow.com by Tom

There is no better way to answer why they are bad than referring to the PHP documentation on magic_quotes:

• Why did we use Magic Quotes?
• Why to not use Magic Quotes

Also note:

This feature has been DEPRECATED as of PHP 5.3.0. Relying on this feature is highly discouraged.

So why does Wordpress still use Magic Quotes?

Wordpress minimum requirements uses PHP 4.3. Yes, it is absolutely a backwards compatibility reason.

What about the other functions?

I am honestly not sure. Relying on super globals is a very bad idea. This is simply laziness of the Wordpress development team. Perhaps they have more important issues to work on.

NOTE: This answer was originally posted at StackOverflow.com by Levi Morrison