MODx Security Fix [for 0.9.1]

• • 
  • 215 Posts
  netnoise Reply #1, 18 years, 1 month ago

  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.
  
  

    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;
    }
  

  
  
  Write secure snippets!
  Lock down your server!
• • 
  • 93 Posts
  jwtyler Reply #2, 18 years, 1 month ago

  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.
• • 
  • 215 Posts
  netnoise Reply #3, 18 years, 1 month ago

  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.
  Write secure snippets!
  Lock down your server!
• • 
  • 258 Posts
  vbrilon Reply #4, 18 years, 1 month ago

  Groovy! Thanks again for the quick catch.
  I have been touched by His noodly appendage
• • 
  • 258 Posts
  vbrilon Reply #5, 18 years, 1 month ago

  Quote from: jwtyler at Apr 15, 2006, 05:21 PM

  
  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.
  I have been touched by His noodly appendage
• • 
  • 1,010 Posts
  TobyL Reply #6, 18 years, 1 month ago

  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
  

  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?
  
  
  eForm | eForm support | eForm @ wiki
• • MODX Staff
  • 12,272 Posts
  Ryan Thrash Reply #7, 18 years, 1 month ago

  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?
  Ryan Thrash, MODX Co-Founder
  Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me
• • 
  • 11 Posts
  Nuker Reply #8, 18 years, 1 month ago

  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.
  

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

  
  
• • ☆ A M B ☆
  • 24,524 Posts
  sottwell Reply #9, 18 years, 1 month ago

  I just set it to return to the home page. No fuss, no bother.
  
  Studying MODX in the desert - http://sottwell.com
  Tips and Tricks from the MODX Forums and Slack Channels - http://modxcookbook.com
  Join the Slack Community - http://modx.org
• • 
  • 15 Posts
  axiome Reply #10, 18 years, 1 month ago

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

This discussion is closed to further replies. Keep calm and carry on.