XH_isAccessProtected() is broken

A place to report and discuss bugs - please mention CMSimple-version, server, platform and browser version
Post Reply
cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

XH_isAccessProtected() is broken

Post by cmb » Sat Jan 24, 2015 1:55 pm

Hello Community,

I found that the access protection check doesn't work for installations in sub folders of the web root, because the protection check completely ignores $sn, so the files are probed relative to the web root. This check usually fails (because the files don't exists), and so the check reports everything would be okay. :evil:

The bug is supposed to be fixed with r1473.

We should consider to make the protection check more resilient, though. As it is now the parsing of the response is sloppy (just using the ninth character of the response), and too permissive (status 5xx should never be the result of an access protected file). To not further delay the release of XH 1.6.5, I suggest to postpone that to XH 1.7 (and maybe to backport an improvement to XH 1.6.6).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is broken

Post by cmb » Sat Apr 11, 2015 11:11 am

I just learned about get_headers() which should help to simplify XH_isAccessProtected() considerably (without the need to use cURL or another extension).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is still broken

Post by cmb » Fri May 01, 2015 12:15 am

cmb wrote:The bug is supposed to be fixed with r1473.
Unfortunately, not really. Requesting http://example.com/?&sysinfo works fine, but http://example.com/index.php?&sysinfo doesn't. Obviously, "index.php" has to be stripped out from $sn. I suggest the following bugfix for XH 1.6.7:

Code: Select all

Index: cmsimple/adminfuncs.php
===================================================================
--- cmsimple/adminfuncs.php	(revision 1581)
+++ cmsimple/adminfuncs.php	(working copy)
@@ -184,7 +184,8 @@
     $stream = fsockopen($host, $_SERVER['SERVER_PORT'], $errno, $errstr, 5);
     if ($stream) {
         stream_set_timeout($stream, 5);
-        $request = "HEAD  $sn$path HTTP/1.1\r\nHost: $host\r\n"
+        $root = preg_replace('/index\.php$/', '', $sn);
+        $request = "HEAD  {$root}{$path} HTTP/1.1\r\nHost: $host\r\n"
             . "User-Agent: CMSimple_XH\r\n\r\n";
         fwrite($stream, $request);
         $response = fread($stream, 12);
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is broken

Post by cmb » Fri May 01, 2015 3:21 pm

For XH 1.7 I suggest to switch to get_headers() and to expect only 4xx response codes for verifying the access protection (see above for the reasoning). You can find the proposed changes in the access-protection branch (only XH_accessProtected is affected).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is broken

Post by cmb » Mon Jun 01, 2015 11:41 am

cmb wrote:For XH 1.7 I suggest to switch to get_headers() and to expect only 4xx response codes for verifying the access protection
Done (r1620).
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is broken

Post by cmb » Wed Jun 03, 2015 11:40 am

cmb wrote:For XH 1.7 I suggest to switch to get_headers() [...]
I just learned that get_headers() requires allow_url_fopen to be enabled. That might be a problem on several shared hosting webspaces, so we might consider to revert the change, or to provide a fallback. Using cURL might be an option, too, but that might also not be available. Yet another option would be to skip the check, if allow_url_fopen is not available, and to make allow_url_fopen a soft requirement for XH 1.7.

Thoughts?

PS: Using get_headers() or cURL() instead of the XH 1.6 solution has the advantage that HTTPS could be supported.

PPS: Done s/allow_furl_open/allow_url_fopen/
Christoph M. Becker – Plugins for CMSimple_XH

cmb
Posts: 14225
Joined: Tue Jun 21, 2011 11:04 am
Location: Bingen, RLP, DE
Contact:

Re: XH_isAccessProtected() is broken

Post by cmb » Mon Jun 15, 2015 6:14 pm

cmb wrote:I suggest the following bugfix for XH 1.6.7: [...]
Done (r1646, r1647).
Christoph M. Becker – Plugins for CMSimple_XH

Post Reply