next »

[netnoise]

This version of document.parser.inc.php fixes some issues in MODx which were published on yesterday.
Please update your current 0.9.1 MODx installations as soon a possible.

How to patch
Open manager/includes/document.parser.class.inc.php in your favourite text editor and replace the function "getDocumentIdentifier" with the code below.

Code:

  function getDocumentIdentifier($method) {
    // function to test the query and find the retrieval method
    $docIdentifier= $this->config['site_start'];
    switch($method) {
      case "alias" :
        $docIdentifier= $this->db->escape($_REQUEST['q']);
      break;
      case "id" :
        if(!is_numeric($_REQUEST['id'])) {
          $this->messageQuit("ID passed in request is NaN!");
        } else {
          $docIdentifier= intval($_REQUEST['id']);
        }
      break;
      default :
      break;
    }
    return $docIdentifier;
  }

[jwtyler]

I must have missed it. What exactly was the security issue? Repatching all the paches into the parser is not something I look forward to.

[netnoise]

I am a non windows user myself (but recoded it to DOS CR/LF as the original one is ;-)

Since the document.parser.class.inc.php you posted includes a bunch of other changes slated for the next release

Oupsie, please see updated posting on top.

[vbrilon]

Groovy! Thanks again for the quick catch.

[vbrilon]

I must have missed it. What exactly was the security issue? Repatching all the paches into the parser is not something I look forward to.

No need to do that. Just replace the one function above.

[TobyL]

Nice one, thank you.

Can anyone tell me where the message "ID passed in request is NaN!" will find a place in the language file?  It will be in there at some stage won't it?  I'd like to make that update in an international installation without hardcoding the mesage in the parser class.

Oh, and a small coding question.  Does the

Code:

default :
      break;

server any purpose?  In my simple brain it doesn't make any difference to the flow if you leave it out so why is it there? Am I missing something?

[rthrash]

I think the default case is required. And thank you for pointing out the messages should probably go in the language files... can you log that  in our Bug (and Support/Feature request tracker), please?

[Nuker]

if user type ./index.php?id=24blablabla in address field then it's get error message about NaN resource.

May'be this code right?  User get only 404 page.

Code:

/*cut*/
     if(!is_numeric($_REQUEST['id'])) {
      $docIdentifier= 0;
/*paste*/

[sottwell]

I just set it to return to the home page. No fuss, no bother.

[axiome]

On special version for free.fr (ModX v0.9.O_Free_Edition), this bug exist ?

[netnoise]

What is "ModX v0.9.O_Free_Edition"

[sottwell]

I think it's an edition where the installer was patched to handle the French mysql error messages for the French "Free" hosting service. Davidm knows more about it.

[Guillaume]

It is right, Sottwell.

Free is a french hoster which hosts website for free. The space drive is big (1Go I think). There are some limitations and most of the CMS (or other php site build) don't work naturally with Free. So it is necessary to patch this application.

[axiome]

Thank you very much for your responses.
I confirm, Free is a host and it offer 1 Go like web space.
Sorry for my english

[davidm]

I think it's an edition where the installer was patched to handle the French mysql error messages for the French "Free" hosting service. Davidm knows more about it.

You can find the Free Edition thread here. Of course, it's in french, but you'll see there that this version has been downloaded almost 600 times....

Actually the problem with MySQL error message happened with all french host set up to display error message in French (with is now solved with the mysql Errno fix) ... Free was another problem which you helped me fix, Susan :
chmod "locked" by hosting at 700 and 644 : possible to run MODx ?

You might not remember but you helped me fix all the includes path, a pretty daunting task which I never updated for 0.9.1

Would it be fair to guess that patching MODx 0.9.0 document.parser.class is perfectly safe ?
Didn't check but was there a change here between 0.9.0 and 0.9.1 ?

[netnoise]

If you're currently using version 0.9.0, you should also apply this patch, to avoid a XSS vulnerability that has been fixed in 0.9.1.

[davidm]

Thanks for the tip Timon, I had forgotten about this...

I'll post something to have MODx 0.9.0 "Free Edition" users patch their installs.
And I'll patch the package so that new downloads are covered

[sottwell]

Would it be fair to guess that patching MODx 0.9.0 document.parser.class is perfectly safe ?
Didn't check but was there a change here between 0.9.0 and 0.9.1 ?

It's not that difficult to patch any version. The pre-.9 versions use a slightly different function, but the switch is much the same, and it's not at all hard to add the validity checks. I did it a bit differently; instead of the cryptic NaN message I just throw the would-be hacker back to the home page. I would feel better about if if the security gurus would verify that this will solve the problem.

FOR PRE-0.9x INSTALLS ONLY!!!! DO NOT APPLY THIS PATCH TO A 0.9X INSTALLATION!!!

Code:

function getDocumentIdentifier($method) {
// function to test the query and find the retrieval method
switch($method) {
case "alias" :
return mysql_escape_string($_REQUEST['q']);
break;
case "id" :
  if(is_numeric($_REQUEST['id'])) {
    return $_REQUEST['id'];
} else {
    return $this->config['site_start'];
}
break;
case "none" :
return $this->config['site_start'];
break;
default :
return $this->config['site_start'];
}
}

[netnoise]

Even if it sounds paranoic it's important to use:

Code:

return intval($_REQUEST['id']);

[sottwell]

Even if it sounds paranoic it's important to use

Code:

return intval($_REQUEST['id']);

So that should be

Code:

case "alias" :
return intval($_REQUEST['q']);
break;