WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 20 months ago

#17856 new enhancement

magic_quotes_gpc future-proof enhancements

Reported by: troydavisson Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Bootstrap/Load Keywords: has-patch
Focuses: Cc:

Description

As is well documented across the Internet, the magic_quotes_gpc feature is going away in future versions of PHP. WordPress has historically automatically escaped _GET, _POST, _REQUEST and _COOKIE input from users, even if the server doesn't have magic_quotes_gpc turned on. Regardless of the reasons for this, having a way to move forward seems absolutely necessary.

Current issues related to this include (among others):

  • maintaining backwards compatibility for those plugin developers who depend on WordPress handling this escaping for them
  • giving plugin developers a way to help put magic_quotes_gpc in the past
  • giving developers access to the original super globals
  • making these super global values read-only so that poorly written plugins/themes don't cause conflicts and problems for other plugins/themes

Attached is a patch which I believe handles this effectively without causing any backwards compatibility issues.

This patch introduces 5 new getter functions for wordpress:

  • wp_input_get()
  • wp_input_post()
  • wp_input_get_post()
  • wp_input_cookie()
  • wp_input_server()

When WordPress first loads, these 5 functions grab the original copies of their respective super globals, undo magic_quotes if it's turned on and then makes the values accessible in a read-only way.

Moving forward, plugin developers can be encouraged to use, for example, wp_input_get('name') rather than $_GETname? . In addition to giving developers a migration path away from the forced magic_quotes_gpc behavior, additional security filters could be done on the given values for further protection.

Attachments (1)

magic_quotes.diff (4.0 KB) - added by troydavisson 4 years ago.

Download all attachments as: .zip

Change History (7)

@troydavisson4 years ago

comment:1 @troydavisson4 years ago

  • Cc troy.davisson@… added

comment:2 @johnbillion4 years ago

In your patch, calling each of the wp_input_*() functions once (from wp_input_init()) just to populate their static variables seems pointless. Why not populate the static variable the first time each function is called?

There should be a way to access a deep value of a multidimensional array. I've seen CMSes and frameworks that allow a pseudo array key notation in the form of

wp_input_post( 'foo[bar][baz]' )

and others that allow for an arbitrary number of arguments in the form of

wp_input_post( 'foo', 'bar', 'baz' );

in order to access $_POST['foo']['bar']['baz'].

comment:3 @troydavisson4 years ago

John, thanks for your feedback. I realize now that I didn't do a great job of explaining the my rationale for how I put together the code.

The attached patch/diff adds code to 2 primary files. The change to wp-settings.php allows for those functions to pre-load the super global data into static arrays. I jumped back and forth between using basic functions or if I should extend the WP class. In the end, using regular functions made the most sense (although it made the code in those functions a bit ugly). The new "wp_input_init()" function is called in wp-settings.php just after the formatting.php file is included (due to the dependency on the stripslashes_deep function) but before "wp_magic_quotes()" is called which is where WordPress goes through and force changes the values of these super globals.

So, populating the static variables ahead of time allows those functions to grab the original values of those variables:

  1. prior to WordPress force-adding magic quotes (the behavior this improvement is supposed to help phase out within WordPress over time)
  2. prior to plugins being loaded which have the ability to alter the values within those arrays. I've run into numerous cases in the past few months where a plugin installed on a WordPress site has either overridden a $_GET variable, completely unset $_GET or has manually overridden one of the $_SERVER values (like the User-Agent)

To summarize, this patch is intended to allow addon developers the access to a read-only version of the original values.

Great suggestion regarding the nested values. I hadn't considered that use-case but I agree that it makes sense. I'll upload an updated patch that covers that.

comment:6 @nacin20 months ago

  • Component changed from General to Bootstrap/Load
Note: See TracTickets for help on using tickets.