WP Automatically escaping GET and POST etc globals

  • Mick P.

    (@mick-p)


    12 years, 7 months ago

    Hello again WP support staff 🙂

    Last night I began noticing some oddness with $_GET and friends under a WP plugin page I am working on.

    Thing is, the contents were being escaped, SQL insertion style. Looked like PHP Magic Quotes. Sure enough, Magic Quotes ™ was on to my surprise. But things were weird, I would strip the slashes from the parameter I was interested in at the top of the plugin, and then later they would come back…

    Now I am not a PHP guru, I use it because I am a C++ guru more or less, so it’s familiar to me syntax wise (and because all of the cool apps do) so I assumed it was simply a scoping deal. I needed to access the parameter in lots of ways and did not want to institute a shadow global for tracking it, so I labored to disable Magic Quotes.

    Now quotes are off, but it is as if WP has its own Magic Quotes regime, as if the original was not bad enough (see the Wikipedia Magic Quotes article)

    It seems like WP is undoing MQ if necessary and then doing its own automatic escaping, probably mysql_real_esc… to _GET and friends at some arbitrary point within the execution timeline.

    A) I thought everyone agreed MQ is a bad strategy, PHP 5.4 removes them, and “the codex” is persistent about escaping your shit, but doesn’t point out that it auto-escapes these things “for” you.

    B) What’s going on here? What strategies are available to me? I don’t like coding around WP, but I’d like to at least know precisely when WP does this and if it does this or not in future updates.

    Thanks again crew!

Viewing 6 replies - 1 through 6 (of 6 total)
  • Aaron Jorbin

    (@jorbin)

    12 years, 7 months ago

    Don’t worry, you’re not going insane and there is work being done to remove this, but in order to not break backwards compatibility this needs to be done smartly. http://core.trac.wordpress.org/ticket/18322 is the trac ticket to follow for this.

    Thread Starter Mick P.

    (@mick-p)

    12 years, 7 months ago

    Thanks for the heads up.

    This seems like something that has only recently received attention.

    I guess what developers need to know is…

    A) When (and to a less extent where in what code file) is the WP slashing done… like action hook wise.

    B) Precisely, as in exactly, what the slashing entails. The slash algorithm, and what gets slashed.

    C) How to detect if it is being done or not.

    Without any of this information its pretty mystical territory.

    Dion Hulse

    (@dd32)

    Meta Developer

    12 years, 7 months ago

    A) I thought everyone agreed MQ is a bad strategy, PHP 5.4 removes them

    I agree, We agree, it’s a bad legacy decision. Back in the days when MQ was the PHP default and “the thing” WordPress (or it’s predecessor? b2) chose to make a consistent environment for itself and Plugins.. A great idea, unfortunately, MQ was the waything at the time.. and it standardised on the “wrong” option.

    This seems like something that has only recently received attention.

    Not so, The ticket is recent, but it’s something which has been in the minds of Core Developers for a long time, It’s something that developers have wanted to remove, but no clear way forward on how to do that.

    A) When (and to a less extent where in what code file) is the WP slashing done… like action hook wise.

    Before init/plugins_loaded/etc, that’s all you need to know (ie. You shouldn’t be running any code before then normally anyway, and the slashing happens during the initial initialization of WordPress – before Plugins are even loaded)

    B) Precisely, as in exactly, what the slashing entails. The slash algorithm, and what gets slashed.

    All $_GET, $_POST, $_COOKIE and $_SERVER superglobals are slashed. $_REQUEST is forced to be $_GET + $_POST only.

    See http://core.trac.wordpress.org/browser/trunk/wp-includes/load.php?rev=18827#L522 for where the magic happens, add_magic_quotes() is simply addslashes() on an array.

    C) How to detect if it is being done or not.

    Assume it’s always being done to said superglobals, There is no way to turn it off, nor detect if WordPress is doing it. This will change in a future release when some action takes place to slowly remove the “functionality”

    Just in case someone gets a brilliant thought: Please never unslash the $_GET/$_POST arrays, Store a copy of it unslashed in your plugin if you wish, but do not modify the globals. WordPress and many plugins expect it to be slashed, and as a result, may introduce unintentional security issues (or just plain weird bugs) in some users environments when their install is doing something yours isn’t (ie. different plugins, themes, etc)

    Thread Starter Mick P.

    (@mick-p)

    12 years, 7 months ago

    Thanks for all the info. Hopefully others will be able to find this one way or another.

    Some food for thought,

    I think there are probably more users who develop private “plugins” then there are viable shared plugins.

    If your plugin is doing something page centric then you can reasonably assume ownership of the _GET/_POST content on a per value basis if its your page’s turn to bat. There should be a list of the get/post values that are used generally (on every page) by WP so not to collide with those.

    To my mind considering the hole WP has dug for itself. I would start by having a simple way for plugins to figure out if WP has finally dropped its MQ regime for good. Even if you don’t have a plan to get out, the sooner that is added the better, so plugins and new development can rely upon it ASAP.

    Then I would add either an action hook into the mix during which the MQs are being installed so a plugin can prevent the slashing of any value they are interested in. Then you can raise a red flag there too if a plugin is claiming ownership of a core WP value — or even if two plugins (hooks) are claiming ownership of the same value. Indeed that might be a very good idea in general (tracking ownership of the values)

    A blacklist global that even administrators can easily setup in wp-config would be good to. For administrators you often need to arrange situations that cannot be dependent on whether a plugin gets to load or not, because sometimes plugins fail, and that would leave your defenses down.

    Ultimately I’d say the very simplest solution is best because this is such a low level matter. Obviously if there has not been any traction after all this time, it seems like its time to do something/anything before things go any further.

    If there isn’t, there should be a Magic Quotes page in the codex, and there should be a mention of the matter in the Plugin development pages. It’s something people can easily not be aware of, because it only comes into play when you need characters in your GET/POST data that would get slashed, and then maybe slashed again, and again. So it has a creep quality that should warrant special attention.

    Dion Hulse

    (@dd32)

    Meta Developer

    12 years, 7 months ago

    I think there are probably more users who develop private “plugins” then there are viable shared plugins.

    If your plugin is doing something page centric then you can reasonably assume ownership of the _GET/_POST content on a per value basis if its your page’s turn to bat.

    And that should always be avoided, even just for consistency between plugins, You can’t just assume that a page is “your plugin’s” page, thanks to the flexibility of WordPress, you can always have other plugins running on a certain request (if they should be running on that request or not is another thing).

    There should be a list of the get/post values that are used generally (on every page) by WP so not to collide with those.

    FWIW, the ones that generally cause problems are those listed as public query vars for WP_Query. IMHO, There’s no good reason not to prefix your query vars (as well as functions) with a consistent prefix.

    I would start by having a simple way for plugins to figure out if WP has finally dropped its MQ regime for good.

    And that’ll be done when #18322 is implemented, There’s no real point in making a hard and fast decision right this instant before it’s been decided as to how to go forward with it. If the wrong decision is made, it’s one that will have to be carried for the next 7 years. Planning is key.

    I would expect 3.4 or 3.5 might make some head way into the issue, It’s not a recent regression, it’s something that’s always been, so there’s time to plan and attack it in the correct manner.

    As for documentation, If you find a Codex page aimed at plugin development which doesn’t make reference to the fact that data is escaped, It’s a wiki, please feel free to update/clarify any points you see. As long as you’re factual and unemotional about the issue at hand, it’ll be greatly appreciated by other new developers.

    Thread Starter Mick P.

    (@mick-p)

    12 years, 6 months ago

    Hats off to WP. I think it’s probably the classiest CMS platform for websites going. It’s built around publishing pages which is something about the internet which is probably not going anywhere, and probably for good reason, and is not crowded with nonsense.

    It would be interesting if it became a de facto standard for publishing websites at some point. Which would I think mean standardizing/sanitizing around a lot of the pitfalls that are out there.

    As for the wp-admin pages (which I am more concerned about) WP has now a concept of “screens” and therefore the plugin that generates the screen should be the owner of the screen. Other plugins can augment that screen, but they should be expected to evolve alongside the plugin. The reader side is pretty simple I am assuming. I try to lure readers into the content generation side of the site by offering more interactive goodies inside.

    As for prefixing get/post data, that’s not so bad for POST. But for GET that just makes the URLs crazier. And often POST/GET overlap. Therefore I don’t think that’s practical. Therefore there should be a mechanism for at least recognizing collisions where they exist so that developers can work around them. If WP can enforce that somehow, then it would be doing some good in terms of stabilizing the environment instead of doing bad, like by clobbering GET/POST data with slashes 🙂

    Point 3. The MQ detection/circumvention opportunities for plugins should be decoupled from whatever solution is ultimately prescribed for WP as a hole. The ultimate goal I am assuming is to remove the slashing entirely. It’s not like it’s hard to slip in a single off/on status check that says that the slasher has finally been defeated.

    Until that’s in there any change that does away with MQs will mean all Plugins prior to the introduction of such a check can be assumed to be broken or suspect (depending on if they need inputs that would be slashed)

Viewing 6 replies - 1 through 6 (of 6 total)
  • The topic ‘WP Automatically escaping GET and POST etc globals’ is closed to new replies.