Changeset 29797

Timestamp:

09/30/2014 02:03:49 PM (10 years ago)

Author:

boonebgorges

Message:

Improve parameter sanitization in WP_Date_Query::build_query().

• Don't run non-numeric values through intval() for sanitization; this transforms them into 1s and 0s, which can cause unintended results.
• Be more generous about numeric array keys (don't require 0 and 1) in BETWEEN and NOT BETWEEN clauses.

Fixes #29801.

Location:

trunk

Files:

• src/wp-includes/date.php (modified) (2 diffs)
• tests/phpunit/tests/date/query.php (modified) (14 diffs)

trunk/src/wp-includes/date.php

@@ -314 +314 @@
             case 'IN':
             case 'NOT IN':
-                return '(' . implode( ',', array_map( 'intval', (array) $value ) ) . ')';
+                $value = (array) $value;
+
+                // Remove non-numeric values.
+                $value = array_filter( $value, 'is_numeric' );
+
+                if ( empty( $value ) ) {
+                    return false;
+                }
+
+                return '(' . implode( ',', array_map( 'intval', $value ) ) . ')';
 
             case 'BETWEEN':
             case 'NOT BETWEEN':
-                if ( ! is_array( $value ) || 2 != count( $value ) || ! isset( $value[0] ) || ! isset( $value[1] ) )
+                if ( ! is_array( $value ) || 2 != count( $value ) ) {
                     $value = array( $value, $value );
+                } else {
+                    $value = array_values( $value );
+                }
+
+                // If either value is non-numeric, bail.
+                foreach ( $value as $v ) {
+                    if ( ! is_numeric( $v ) ) {
+                        return false;
+                    }
+                }
 
                 $value = array_map( 'intval', $value );
@@ -326 +345 @@
 
             default;
+                if ( ! is_numeric( $value ) ) {
+                    return false;
+                }
+
                 return (int) $value;
         }

trunk/tests/phpunit/tests/date/query.php

@@ -230 +230 @@
     }
 
+    /**
+     * @ticket 29801
+     */
     public function test_build_value_compare_in() {
         $q = new WP_Date_Query( array() );
@@ -239 +242 @@
         // Single non-integer
         $found = $q->build_value( 'IN', 'foo' );
-        $this->assertSame( '(0)', $found );
+        $this->assertFalse( $found );
 
         // Array of integers
@@ -247 +250 @@
         // Array containing non-integers
         $found = $q->build_value( 'IN', array( 1, 'foo', 7 ) );
-        $this->assertSame( '(1,0,7)', $found );
-    }
-
+        $this->assertSame( '(1,7)', $found );
+    }
+
+    /**
+     * @ticket 29801
+     */
     public function test_build_value_compare_not_in() {
         $q = new WP_Date_Query( array() );
@@ -259 +265 @@
         // Single non-integer
         $found = $q->build_value( 'NOT IN', 'foo' );
-        $this->assertSame( '(0)', $found );
+        $this->assertFalse( $found );
 
         // Array of integers
@@ -267 +273 @@
         // Array containing non-integers
         $found = $q->build_value( 'NOT IN', array( 1, 'foo', 7 ) );
-        $this->assertSame( '(1,0,7)', $found );
+        $this->assertSame( '(1,7)', $found );
     }
 
@@ -278 +284 @@
 
     /**
-     * @todo This is a bug. Should return false when non-numeric
+     * @ticket 29801
      */
     public function test_build_value_compare_between_single_non_numeric() {
@@ -284 +290 @@
 
         $found = $q->build_value( 'BETWEEN', 'foo' );
-        $this->assertSame( '0 AND 0', $found );
-    }
-
-    /**
-     * @todo This is a bug
+        $this->assertFalse( $found );
+    }
+
+    /**
+     * @ticket 29801
      */
     public function test_build_value_compare_between_array_with_other_than_two_items() {
@@ -294 +300 @@
 
         $found = $q->build_value( 'BETWEEN', array( 2, 3, 4 ) );
-
-        // array_map( 'intval', array(
-        //     array( 2, 3, 4 ),
-        //     array( 2, 3, 4 ),
-        // ) );
-        $this->assertSame( '1 AND 1', $found );
-    }
-
-    /**
-     * @todo This is a bug
+        $this->assertFalse( $found );
+    }
+
+    /**
+     * @ticket 29801
      */
     public function test_build_value_compare_between_incorrect_array_key() {
@@ -313 +314 @@
         ) );
 
-        // array_map( 'intval', array(
-        //     array( 2 => 4, 3 => 5 ),
-        //     array( 2 => 4, 3 => 5 ),
-        // ) );
-        $this->assertSame( '1 AND 1', $found );
-    }
-
+        $this->assertSame( '4 AND 5', $found );
+    }
+
+    /**
+     * @ticket 29801
+     */
     public function test_build_value_compare_between_array_contains_non_numeric() {
         $q = new WP_Date_Query( array() );
 
         $found = $q->build_value( 'BETWEEN', array( 2, 'foo' ) );
-        $this->assertSame( '2 AND 0', $found );
+        $this->assertFalse( $found );
     }
 
@@ -341 +341 @@
     }
 
+    /**
+     * @ticket 29801
+     */
     public function test_build_value_compare_not_between_single_non_numeric() {
         $q = new WP_Date_Query( array() );
 
         $found = $q->build_value( 'NOT BETWEEN', 'foo' );
-        $this->assertSame( '0 AND 0', $found );
-    }
-
-    /**
-     * @todo This is a bug
+        $this->assertFalse( $found );
+    }
+
+    /**
+     * @ticket 29801
      */
     public function test_build_value_compare_not_between_array_with_other_than_two_items() {
@@ -355 +358 @@
 
         $found = $q->build_value( 'NOT BETWEEN', array( 2, 3, 4 ) );
-
-        // array_map( 'intval', array(
-        //     array( 2, 3, 4 ),
-        //     array( 2, 3, 4 ),
-        // ) );
-        $this->assertSame( '1 AND 1', $found );
-    }
-
-    /**
-     * @todo This is a bug
+        $this->assertFalse( $found );
+    }
+
+    /**
+     * @ticket 29801
      */
     public function test_build_value_compare_not_between_incorrect_array_key() {
@@ -374 +372 @@
         ) );
 
-        // array_map( 'intval', array(
-        //     array( 2 => 4, 3 => 5 ),
-        //     array( 2 => 4, 3 => 5 ),
-        // ) );
-        $this->assertSame( '1 AND 1', $found );
-    }
-
+        $this->assertSame( '4 AND 5', $found );
+    }
+
+    /**
+     * @ticket 29801
+     */
     public function test_build_value_compare_not_between_array_contains_non_numeric() {
         $q = new WP_Date_Query( array() );
 
         $found = $q->build_value( 'NOT BETWEEN', array( 2, 'foo' ) );
-        $this->assertSame( '2 AND 0', $found );
+        $this->assertFalse( $found );
     }
 
@@ -403 +400 @@
 
     /**
-     * @todo This is probably a bug - ought to return false
+     * @ticket 29801
      */
     public function test_build_value_compare_default_value_non_numeric() {
@@ -409 +406 @@
 
         $found = $q->build_value( 'foo', 'foo' );
-        $this->assertSame( 0, $found );
+        $this->assertFalse( $found );
     }