Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

access denied and & issue #3943

Closed
Jimmi08 opened this issue Sep 1, 2019 · 6 comments
Closed

access denied and & issue #3943

Jimmi08 opened this issue Sep 1, 2019 · 6 comments
Labels
status: testing required Someone needs to confirm this issue's existence and write a test to prevent the fix from regressing.

Comments

@Jimmi08
Copy link
Contributor

Jimmi08 commented Sep 1, 2019

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

@Moc Moc added the status: testing required Someone needs to confirm this issue's existence and write a test to prevent the fix from regressing. label Sep 3, 2019
@Deltik
Copy link
Member

Deltik commented Jan 14, 2020

@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.

@Deltik Deltik self-assigned this Jan 14, 2020
@Jimmi08
Copy link
Contributor Author

Jimmi08 commented Jan 14, 2020

@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 Deltik removed their assignment Jan 14, 2020
@Deltik
Copy link
Member

Deltik commented Jan 14, 2020

@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
Copy link
Contributor Author

Jimmi08 commented Jan 15, 2020

@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
Copy link
Member

CaMer0n commented Jan 19, 2020

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
Copy link
Contributor Author

Jimmi08 commented Jan 19, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: testing required Someone needs to confirm this issue's existence and write a test to prevent the fix from regressing.
Projects
None yet
Development

No branches or pull requests

4 participants