e107inc / e107

e107 Bootstrap CMS (Content Management System) v2 with PHP, MySQL, HTML5, jQuery and Twitter Bootstrap. Issue Discussion Room: https://gitter.im/e107inc/e107
https://e107.org
GNU General Public License v3.0
321 stars 214 forks source link

access denied and & issue #3943

Closed Jimmi08 closed 4 years ago

Jimmi08 commented 5 years ago

I got access denied on some urls. I was able to narrow problem to this:

    public function set_request($no_cbrace = true)
    {

        $inArray = array("'", ';', '/**/', '/UNION/', '/SELECT/', 'AS ');
        if (strpos($_SERVER['PHP_SELF'], 'trackback') === false)
        {
            foreach($inArray as $res)
            {  
                if(stristr($_SERVER['QUERY_STRING'], $res))
                 {
                    die('Access denied.');
                }
            }
        }

$_SERVER['QUERY_STRING'] : rt=product/product&product_id=123 failed test: $res : ';'

it looks like stristr test & instead of & No idea what to do.
Not for 100% sure, but I have integrated e107 with Nuke with similar URLs and it worked when the request part was in class2.php not in e107 class. Thanks

Deltik commented 4 years ago

@Jimmi08: How did you find this issue? I can't reproduce it unless I explicitly type & into the query string, which should not happen under normal circumstances.

Jimmi08 commented 4 years ago

@Deltik I use e107 with abantecart system.
It fails with & in URL. No idea what to check if $_SERVER['QUERY_STRING'] value is correct & but stristr is checking & Hm, not sure now if I var_dumped that value or used print_a for that $_SERVER['QUERY_STRING'].

But while this test was in class2.php, it worked.

I fixed it by using e107_class without this test. And I will have the next case with other CMS, so I will see what is causing the problem.

It's one of the e107 advantages, you don't need some bridges, you just include class2.php and you can extend other CMS with e107 features. I know, not standard, but this is the first issue I run with it.

Deltik commented 4 years ago

@Jimmi08: I still don't see how just a & triggers the access denied error.

As for the ; match in the query string, I suspect there may be legitimate uses for it. That matching code was written ~a very long time ago, before e107 v0.8 was created~ on 02 May 2005:

diff --git a/e107_0.7/class2.php b/e107_0.7/class2.php
index fa7b25453..31e96726b 100644
--- a/e107_0.7/class2.php
+++ b/e107_0.7/class2.php
@@ -12,9 +12,9 @@
 |     GNU General Public License (http://gnu.org).
 |
 |     $Source: /cvs_backup/e107_0.7/class2.php,v $
-|     $Revision: 1.116 $
-|     $Date: 2005-05-02 14:39:14 $
-|     $Author: streaky $
+|     $Revision: 1.117 $
+|     $Date: 2005-05-02 17:37:03 $
+|     $Author: stevedunstan $
 +----------------------------------------------------------------------------+
 */

@@ -62,8 +62,16 @@ for ($i = 1; $i <= $num_levels; $i++) {
        $link_prefix .= "../";
 }

-if (!strstr($_SERVER['PHP_SELF'], "trackback") && (strstr($_SERVER['QUERY_STRING'], "'") || strstr($_SERVER['QUERY_STRING'], ";"))) {
-       die("Access denied.");
+$inArray = array("'", ";", "/**/", "/UNION/", "/SELECT/", "AS");
+if (!strstr($_SERVER['PHP_SELF'], "trackback"))
+{
+       foreach($inArray as $res)
+       {
+               if(strstr($_SERVER['QUERY_STRING'], $res))
+               {
+                       die("Access denied.");
+               }
+       }
 }

 if (preg_match("/\[(.*?)\].*?/i", $_SERVER['QUERY_STRING'], $matches)) {
@@ -229,6 +237,7 @@ $pref['htmlarea']=false;

 define("e_SELF", ($pref['ssl_enabled'] ? "https://".$_SERVER['HTTP_HOST'].($_SERVER['PHP_SELF'] ? $_SERVER['PHP_SELF'] : $_SERVER['SCRIPT_FILENAME']) : "http://".$_SERVER['HTTP_HOST'].($_SERVER['PHP_SELF'] ? $_SERVER['PHP_SELF'] : $_SERVER['SCRIPT_FILENAME'])));

+/*
 if($pref['redirectsiteurl'])
 {
        if($e107 -> http_path != $pref['siteurl'])
@@ -241,7 +250,7 @@ if($pref['redirectsiteurl'])
                }
        }
 }
-
+*/

 // Cameron's Mult-lang switch. ==================

Maybe @CaMer0n can provide some input on whether that code is still good to have today.

Jimmi08 commented 4 years ago

@Deltik that's the point While it was in class2.php, it worked. After moving to e107_class.php I run to this issue.

It's not important, but if there is little chance that somebody uses &amp; instead of & in URL, I thought it would be good to mention this. Or it's maybe related to the environment, no idea.
Thanks for your time.

CaMer0n commented 4 years ago

I believe the semi-colon check can be removed. Commit coming shortly. https://stackoverflow.com/questions/2366260/whats-valid-and-whats-not-in-a-uri-query

Jimmi08 commented 4 years ago

Thank you!