Avoid looking for ? when checking for file to be included

Signed-off-by: Michal Čihař <michal@cihar.com>

Author: nijel

index.php

@@ -56,7 +56,7 @@
     && is_string($_REQUEST['target'])
     && ! preg_match('/^index/', $_REQUEST['target'])
     && ! in_array($_REQUEST['target'], $target_blacklist)
-    && Core::checkPageValidity($_REQUEST['target'])
+    && Core::checkPageValidity($_REQUEST['target'], [], true)
 ) {
     include $_REQUEST['target'];
     exit;

libraries/classes/Core.php

@@ -435,12 +435,13 @@ public static function getRealSize($size = 0)
      * checks given $page against given $whitelist and returns true if valid
      * it optionally ignores query parameters in $page (script.php?ignored)
      *
-     * @param string &$page     page to check
-     * @param array  $whitelist whitelist to check page against
+     * @param string  &$page     page to check
+     * @param array   $whitelist whitelist to check page against
+     * @param boolean $include   whether the page is going to be included
      *
      * @return boolean whether $page is valid or not (in $whitelist or not)
      */
-    public static function checkPageValidity(&$page, array $whitelist = [])
+    public static function checkPageValidity(&$page, array $whitelist = [], $include = false)
     {
         if (empty($whitelist)) {
             $whitelist = self::$goto_whitelist;
@@ -452,6 +453,9 @@ public static function checkPageValidity(&$page, array $whitelist = [])
         if (in_array($page, $whitelist)) {
             return true;
         }
+        if ($include) {
+            return false;
+        }
 
         $_page = mb_substr(
             $page,

test/classes/CoreTest.php

@@ -267,9 +267,9 @@ function testArrayRemove()
      *
      * @dataProvider providerTestGotoNowhere
      */
-    function testGotoNowhere($page, $whiteList, $expected)
+    function testGotoNowhere($page, $whiteList, $include, $expected)
     {
-        $this->assertSame($expected, Core::checkPageValidity($page, $whiteList));
+        $this->assertSame($expected, Core::checkPageValidity($page, $whiteList, $include));
     }
 
     /**
@@ -280,12 +280,18 @@ function testGotoNowhere($page, $whiteList, $expected)
     public function providerTestGotoNowhere()
     {
         return array(
-            array(null, [], false),
-            array('export.php', [], true),
-            array('export.php', $this->goto_whitelist, true),
-            array('shell.php', $this->goto_whitelist, false),
-            array('index.php?sql.php&test=true', $this->goto_whitelist, true),
-            array('index.php%3Fsql.php%26test%3Dtrue', $this->goto_whitelist, true),
+            array(null, [], false, false),
+            array(null, [], true, false),
+            array('export.php', [], false, true),
+            array('export.php', [], true, true),
+            array('export.php', $this->goto_whitelist, false, true),
+            array('export.php', $this->goto_whitelist, true, true),
+            array('shell.php', $this->goto_whitelist, false, false),
+            array('shell.php', $this->goto_whitelist, true, false),
+            array('index.php?sql.php&test=true', $this->goto_whitelist, false, true),
+            array('index.php?sql.php&test=true', $this->goto_whitelist, true, false),
+            array('index.php%3Fsql.php%26test%3Dtrue', $this->goto_whitelist, false, true),
+            array('index.php%3Fsql.php%26test%3Dtrue', $this->goto_whitelist, true, false),
         );
     }