[SOLVED] can't browse a certain page

[cmb]

Hello developers,

while testing Pagemanager_XH I had a strange timeout behaviour while clicking the menu link to a page with title 'i'. Thanks to XHdebugmode I was able to locate the error in cms.php line 125. The code is:

Code: Select all

if (sv('QUERY_STRING') != '') {
    $rq = explode('&', sv('QUERY_STRING'));
    if (!strpos($rq[0], '='))$su = $rq[0];
    $v = count($rq);
    for($i = 0; $i < $v; $i++)if(!strpos($rq[$i], '='))$GLOBALS[$rq[$i]] = 'true';
}
 

The problem is, that the for-loop doesn't terminate. IMHO it should, because intval('true')==0, but it doesn't. I've double-checked that.

Okay, you might say, that's not a big problem, cause nobody will title a page 'i' in the real world.

But what is with other globals? E.g. a user might call his page hjs, because his name is Henry John Smith, or su, because he's a unix geek. I've not tested those cases, but IMHO it's perhaps not the best idea to set a global by a query param without a check. At least a malicious attacker could use the ?i to keep a CMSimple server very busy

Christoph

[Martin]

Good news for Henry John Smith and the super-user: it happens only with $i.

If $i is passed as $_GET-var, this old legacy piece of code sets the global variable $i to 'true'. And in PHP's mind the string 'true' is lesser than 1. ( (int)'true or whatever' is 0 ) That's why the loop will never terminate.

If soemone really wants to create a top-level-page (or a plugin) called 'i' - he could use

Code: Select all

if (sv('QUERY_STRING') != '') {
    $rq = explode('&', sv('QUERY_STRING'));
    if (!strpos($rq[0], '='))$su = $rq[0];
    $v = count($rq);
    foreach($rq as $var){
        if(!strpos($var, '=')){ $GLOBALS[$var] = true;  }
     }
} 

or something like that as a quick fix.

Probably there is no real need to fix this immediately. But still - that's something! It points to some really very old lines of code with a somewhat adventurous use of global variables. (Adventurous not in respect of vulnerability, but in respect of teamwork.)

Martin

[cmb]

Hello Martin,

thanks for your fast (and late ) reply.

Good news for Henry John Smith and the super-user: it happens only with $i.

That are really good news! Even if you might introduce a new global $cmb, I don't have to worry

And in PHP's mind the string 'true' is lesser than 1. ( (int)'true or whatever' is 0 ) That's why the loop will never terminate.

But shouldn't the 0 be increased to 1 in the "next" part of for (i.e. $i++), so the loop terminates on $i < $v? I don't quite understand. I should consult php.net.

Probably there is no real need to fix this immediately. But still - that's something!

If the fix is as easy as you've proposed, couldn't it be done for the next release? But I guess you've got it already on the TODO list.

Christoph