We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 8609
    • 607 Posts
    I just read this post - http://heine.familiedeelstra.com/modx-arbitrary-code-execution and it has me a little concerned. I’m about to deploy a crucial website using 1.0.0 and have several sites running on 0.9.6.3. In the article the writer states that he has talked to MODx devs about this security issue that affects 1.0.0 also. Does this mean that there are known security holes in Evo? Is there a fix that can be applied to our sites or should we just wait on Revo for a secure MODx?

    thanks
      • 25663 MODX Staff
      • 12,272 Posts
      Not exactly an unbiased source is it? wink Though it certainly is entertaining to see this type of friendly ribbing coming from the Drupal camp. For the record, I have nothing but praise for the Drupal project, it’s certainly the right solution for lots of sites. Horses for courses.

      At any rate, first you have to be logged into the manager to make the potential exploit work, with a specific MODx configuration. You have to have Snippets wide open to end users who don’t know anything about coding. Then you have to be tricked into visiting another site with malicious code and such a compelling story that it further tricks you into creating a new Snippet with malicious code in it with the runSnippet option checked—an option that I’ve never seen used in a production site. At least in the sites I’ve seen built in recent years, end users content editor roles have not had access to snippets, but I’m sure that doesn’t apply across the board (it should!). Then the rest of the mojo happens and you have to have a social site which allows a lot of user uploads from the front end, which any semi-intelligent human knows you should be running Drupal to do that. Duh!

      In all fairness and kidding aside, we can be more clear about the risk. It’s been discussed and isn’t hidden, but we’ve not shouted from the rooftops about the risks. If you don’t trust your Manager users, you should enable the protection option. It has not been on by default because it causes problems on some server configurations. You absolutely should invest in a properly secured and monitored server with good admins backing you up. Likewise, you can remove the run snippet option out of the parser, which I’m not sure has ever been used by nuking a few lines of code. Specific details on that soon.

      Jason and Shaun are hard at work on Revo and that’s where their time is focused on the develoment and architecture side. Evo is not going anywhere any time soon. It will be maintained, new development will happen, and in fact it’s got more talented devs working on it today than any time in the past. We’ll post some things to help protect more against this for sites you feel will fall into the hands of content editors with full access to all of the Manager and a proclivity for posting random Snippets while visiting sites like MODxHaterPornWarez.com during logged in Manager sessions.
        Ryan Thrash, MODX Co-Founder
        Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me
        • 25663 MODX Staff
        • 12,272 Posts
        We don’t want to hide potential security exploits, why would it be an issue? Though courtesy would have let us put together a patch release before publishing the article.
          Ryan Thrash, MODX Co-Founder
          Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me
          • 25663 MODX Staff
          • 12,272 Posts
          As a quick additional preventative measure, you can remove the runsnippet function in the /manager/includes/document.parser.class.php file. In Evo 1.0, this should be ~lines 1695-1716. In MODx 0.9.6.x it should be almost in the same spot at lines 1691-1717. That said there’s nothing to prevent people from creating a snippet, putting it on a page, executing the snippet while logged in and so on...

          Also please turn on the option to check referrers and do apply the patch in the check referrer function that makes sure your local install starts with your site for local referrers, and not just contains the cleverly tricked-you-into-uploading code:

          --- manager/index.php	2009-09-08 12:33:54.000000000 +0200
          +++ manager/index.php	2009-09-08 12:35:35.000000000 +0200
          @@ -230,7 +230,7 @@ if (isset($modx->config['validate_refere
                   $referer = $_SERVER['HTTP_REFERER'];
           
                   if (!empty($referer)) {
          -            if (!preg_match('/'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
          +            if (!preg_match('/^'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
                           echo "A possible CSRF attempt was detected from referer: {$referer}.";
                           exit();
                       }
            Ryan Thrash, MODX Co-Founder
            Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me
            • 8609
            • 607 Posts
            Thanks for the detailed reply Ryan. I was definitely concerned to read about this from a source outside MODx since I hadn’t read anything about it on the MODx website or forums. I have read about the previous exploit on 0.9.6.2 but thought 0.9.6.3 and 1.0.0 were pretty secure. So this was relatively alarming, and must be even more so for people who are using MODx but don’t necessarily keep their finger on its pulse constantly.

            Nonetheless, perhaps we need a better way of letting the community know about these kinds of issues, and not just through the forums. I know a lot of people find the forums overwhelming to navigate and don’t monitor them closely, so it might help to have an updates area on the main website where information like this is released. I know from this from my own experience and from emails that I get from my blog readers. So mission critical info like security issues/potential security issues/snippet updates etc perhaps deserve a news bulletin section of their own outside of the forums.

            Just my 2c.
              • 8609
              • 607 Posts
              Case in point to my little idea above. This info really should not just be in the forums, but rather should be in a in an updates/security issues section of its own on the main website where it’s readily accessible.

              Quote from: rthrash at Sep 08, 2009, 10:59 PM

              As a quick additional preventative measure, you can remove the runsnippet function in the /manager/includes/document.parser.class.php file. In Evo 1.0, this should be ~lines 1695-1716. In MODx 0.9.6.x it should be almost in the same spot at lines 1691-1717. That said there’s nothing to prevent people from creating a snippet, putting it on a page, executing the snippet while logged in and so on...

              Also please turn on the option to check referrers and do apply the patch in the check referrer function that makes sure your local install starts with your site for local referrers, and not just contains the cleverly tricked-you-into-uploading code:

              --- manager/index.php	2009-09-08 12:33:54.000000000 +0200
              +++ manager/index.php	2009-09-08 12:35:35.000000000 +0200
              @@ -230,7 +230,7 @@ if (isset($modx->config['validate_refere
                       $referer = $_SERVER['HTTP_REFERER'];
               
                       if (!empty($referer)) {
              -            if (!preg_match('/'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
              +            if (!preg_match('/^'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
                               echo "A possible CSRF attempt was detected from referer: {$referer}.";
                               exit();
                           }

                • 16183
                • 1,390 Posts
                Quote from: rthrash at Sep 08, 2009, 10:59 PM

                As a quick additional preventative measure, you can remove the runsnippet function in the /manager/includes/document.parser.class.php file. In Evo 1.0, this should be ~lines 1695-1716. In MODx 0.9.6.x it should be almost in the same spot at lines 1691-1717. That said there’s nothing to prevent people from creating a snippet, putting it on a page, executing the snippet while logged in and so on...

                Also please turn on the option to check referrers and do apply the patch in the check referrer function that makes sure your local install starts with your site for local referrers, and not just contains the cleverly tricked-you-into-uploading code:

                --- manager/index.php	2009-09-08 12:33:54.000000000 +0200
                +++ manager/index.php	2009-09-08 12:35:35.000000000 +0200
                @@ -230,7 +230,7 @@ if (isset($modx->config['validate_refere
                         $referer = $_SERVER['HTTP_REFERER'];
                 
                         if (!empty($referer)) {
                -            if (!preg_match('/'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
                +            if (!preg_match('/^'.preg_quote(MODX_SITE_URL, '/').'/i', $referer)) {
                                 echo "A possible CSRF attempt was detected from referer: {$referer}.";
                                 exit();
                             }


                Is that a typo on line 3?
                $modx->config['validate_refere
                should be
                $modx->config['validate_referer']
                I think...

                Cheers/k
                  • 16183
                  • 1,390 Posts
                  Quote from: einsteinsboi at Sep 08, 2009, 11:07 PM

                  Nonetheless, perhaps we need a better way of letting the community know about these kinds of issues, and not just through the forums. I know a lot of people find the forums overwhelming to navigate and don’t monitor them closely, so it might help to have an updates area on the main website where information like this is released. I know from this from my own experience and from emails that I get from my blog readers. So mission critical info like security issues/potential security issues/snippet updates etc perhaps deserve a news bulletin section of their own outside of the forums.

                  Just my 2c.

                  Totally agree. I have argued for this before (during the reflect exploit). The counter argument was that folks should subscribe to security feeds and also read the news feeds within the manager. IMHO, I still think that is not enough...Anyway...just my 2p

                  cheers/k
                    • 25663 MODX Staff
                    • 12,272 Posts
                    We’ll figure out a spot for important announcements on the main home page in the near future.
                      Ryan Thrash, MODX Co-Founder
                      Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me
                      • 11956
                      • 1 Posts
                      Perhaps not unbiased; I hardly know anything about modx. I did try to not write a modx vs. Drupal post. Rest assured, I found countless security issues in Drupal as well (including a PHP code execution vulnerablity via CSRF @ http://drupal.org/node/113935).

                      I’m happy to see planned improvements to the security process.

                      As it is now, I feel that the HTTP_REFERER check bypass is moot; the functionality isn’t enabled by default and the poc should work on a default modx site regardless of the patch.

                      [...]We’ll post some things to help protect more against this for sites you feel will fall into the hands of content editors with full access to all of the Manager and a proclivity for posting random Snippets while visiting sites like MODxHaterPornWarez.com during logged in Manager sessions.

                      Unfortunately, the victim doesn’t need to do anything more than just visiting the evil site (exploit.html in the proof of concept) while logged in to the manager. The hidden iframe submits the snippet in the background. I’ll try to make this clearer in the post.

                      IMO it is important to enable CSRF protection by default. As far as I can tell, there’s a forum post in the security section on 0.9.6 that tells you this, but I don’t think this is very well documented for new end users. Better be (more) secure by default I’d think.

                      Apart from enabling CSRF protection by default, I would recommend adding tokens to requests and checking them; this way GET requests are also secured and you can use the manager with ZoneAlarm and a certain Norton product (whose name we shouldn’t mention). I know this takes resources, so it’s up to you to balance the issues.

                      Heine