Permalink
Browse files

Enforce a minimum of two arguments for wpdb::prepare(). The first arg…

…ument is the query (or fragment thereof), which is required. Additional arguments are values to substitute into placeholders.

This will generate E_WARNINGs for insufficient arguments when prepare() is called with no additional arguments. This should discourage improper uses of prepare() under the guise of safely running a query.

props xknown. fixes #22262.



git-svn-id: http://core.svn.wordpress.org/trunk@22429 1a063a9b-81f0-0310-95a4-ce76da25c4cd
  • Loading branch information...
nacin committed Nov 7, 2012
1 parent 3a86c87 commit e588812a498f0d0f8321a7d61b0b67ea59ea3c43
Showing with 1 addition and 1 deletion.
  1. +1 −1 wp-includes/wp-db.php
View
@@ -987,7 +987,7 @@ function escape_by_ref( &$string ) {
* @return null|false|string Sanitized query string, null if there is no query, false if there is an error and string
* if there was something to prepare
*/
- function prepare( $query = null ) { // ( $query, *$args )
+ function prepare( $query, $args ) {
if ( is_null( $query ) )
return;

5 comments on commit e588812

Ops,
A lot of plugins not using "$args". Maybe you can do it so :

function prepare( $query, $args=null ) {}

Owner

nacin replied Nov 20, 2012

Per the commit message, we are deliberately causing E_WARNING to be triggered when $args is omitted. They are passing insufficient arguments to the point where they risk security issues doing so.

Oh you are right, security is important ;) thanks for time and develop 👍

IMHO the first minor release should contain backward compatible signatures. It would have been the best to make it optional in the 3.6 and then make it hard in 3.6.1. With an announcement or and (hidden) obsolete warning (E_NOTICE) you could've avoid the trouble most of the users have and give the security at once.

Owner

nacin replied Dec 12, 2012

We thought about it. We probably could have had it generate a notice in 3.5 and a warning in 3.6, but I was incredibly torn by the idea of shipping a notice that most developers wouldn't even see (let alone give themselves a chance to ignore it) when this is, at its heart, a potential security issue. Issuing a warning seemed like the most responsible thing to do, despite the (relatively minor) pain it'll cause.

I did a write-up here: http://make.wordpress.org/core/2012/12/12/php-warning-missing-argument-2-for-wpdb-prepare/.

A side-note, we've gotten very good (I say this facetiously) about accidentally breaking plugins that were doing something wrong in a major release, only to fix the issue in the next minor release after all of the plugins have updated for it. Happened with JavaScript enqueueing hooks in both 3.2 and 3.4. This was indeed a deliberate change, but there's nothing preventing us from moving to a notice in 3.5.1, then back to a warning in 3.6 again (hopefully giving developers some cover to make changes).

Please sign in to comment.