Subscribe: RSS
  • MODX now has its security team.
    But let me quote what it says:

    Extras Security Policy

    The MODX Security Team is not responsible for Extras for MODX, nor any security issues found within them. When a vulnerability is found in a MODX Extra, the Security Team will be notified and the author will be contacted regarding the issue. The author will be given a reasonable deadline by which to fix the issue. If the deadline is not met, the Extra will be removed from the official MODX Extras repository and a public advisory against the Extra will be released. Deadlines will be set by the Security Team in accordance with the severity of the issue. MODX or the Security Team will not audit or review Extras hosted on any non-official Provider.

    Before the real Security Standards have been noted clearly, I like to invite you guys to share all of your knowledge here about how to close the security holes, as a 3PC developer.

    Let me start my part:
    1. You may start to learn this deeply by reading a good book: PHP Architect’s Guide to PHP Security.
    2. Bookmark and dig this: http://phpsec.org/projects/guide/
    3. Well, start google about this matter. grin
    4. For some quick recommendations, please see below.

    1. Always sanitize your input
    Do not trust your user!

    a. Form
    You should always sanitize your input form in any way, when the form is submitted.
    It can be your search box, your login/register form, your contact form, your comment form, whatever.
    When submitting, the FIRST or FOREMOST process is sanitizing.
    Others come afterward.

    b. Snippet’s parameters
    Why do you think that developers can not be wrong?

    MODX has its sanitizing APIs: sanitize dan sanitizeString
    <?php // highlight
    
    $modx->sanitize($mixed); // array
    $modx->sanitizeString($string); // only string
    

    BUT, if you dig the code, it sanitize ASCII regex only.
    You should create your own method to fit your characters if they are not ASCII texts.
    Revo still misses the even trigger for TransAlias plugin like Evo has.

    2. Do Not Pass Any Requests Directly to the Class
    If you apply an OOP style to your code, please consider this.

    Always sanitize first before your snippet code passes them to any object.
    /* ex.1 */
    <?php //highlight
    
    $object = new MyClass($modx, $scriptProperties);  // still having the raw values, absolutely dangerous!
    $object->submitMyForm($_POST);                    // absolutely dangerous!
    $object->property = $_POST['property'];           // absolutely dangerous!
    


    /* ex.2 */
    <?php //highlight
    $sanitizedPosts = $modx->sanitize($_POST);
    $_POST = array();
    $object->submitMyForm($sanitizedPosts); // recommended!
    


    /* ex.3 */
    <?php //highlight
    $sanitizedGets = $modx->sanitize($_GET);
    $_GET= array();
    $object->submitMyForm($sanitizedGets); // recommended!
    


    /* ex.4 */
    <?php //highlight
    $scriptProperties['param'] = !empty($_GET['param']) ? $modx->sanitizeString($_GET['param']) : '';  // recommended!
    $config['param'] = $modx->getOption('param', $scriptProperties, 'defaultValue');
    


    /* ex.5 */
    <?php //highlight
    
    class MyClass {
    
        // dangerous
        public function myFunction () {
            $title = $_POST['title'];
            echo $title;
        }
    
        // recommended
        public function myFunction2 ($sanitizedPost) {
            $title = $sanitizedPost['title'];
            echo $title;
        }
    }
    


    3. Always sanitize your Ajax file
    Since each of controller or ajax processor files is like an independent file, you should make a ’check code’ ON TOP of the file.
    You can use:

    a. Access check.
    /* ex.6 */
    <?php
    if (!defined('IN_MANAGER_MODE')) {
        return;
        // or
        // die();
    }
    


    b. Sanitize file using MODX’s API
    /* ex.7 */
    <?php
    // ... include the class file here ...
    $modx = new modX();
    $sanitizedRequests = $modx->sanitize($_REQUEST); // recommended, will be applied to all $_GET, $_POST, $_COOKIE
    $_REQUEST = array();
    $_REQUEST = sanitizedRequests;
    
    ... proceed ...
    



    About the file inclusion and if this is about the front-end AJAX, you can set the ajax processor using the MODX’s resource.
    Use a blank template to that resource, and make a snippet that calls the real AJAX processor file.
    That will make the $modx object available to that processor.
    You do not need to include the MODX’s core file again => that will make the direct access to the file become PHP Fatal Error! tongue



    So far, these are my thoughts.
    I need to share this, because I also use other 3PC extras that are not built by myself.
    I hope you can understand this situation, and even better share your experiences here.

    For the Secure MODX!
      Rico
      Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
      MODx is great, but knowing how to use it well makes it perfect!

      www.virtudraft.com

      Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

      Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

      Maintainter/contributor of Babel

      Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
    • Thanks for this goldsky! Much appreciated smiley
      • your welcome, mary.
        Let me continue a bit.

        Sanitize Your FormIt with a Hook

        FormIt has Hook option.
        We can create a snippet that can be used as our sanitizer here.
        This is my example, you can improve it again for your need, for this example I call this hook as mySnippet

        <?php
        
        /**
         * for FormIt's hook
         * http://modxcms.com/forums/index.php/topic,60716.msg345837.html#msg345837
         */
        if (get_magic_quotes_gpc ()) {
            if (!function_exists('stripslashes_gpc')) {
        
                function stripslashes_gpc(&$value) {
                    $value = stripslashes($value);
                }
        
            }
            array_walk_recursive($_GET, 'stripslashes_gpc');
            array_walk_recursive($_POST, 'stripslashes_gpc');
            array_walk_recursive($_COOKIE, 'stripslashes_gpc');
            array_walk_recursive($_REQUEST, 'stripslashes_gpc');
        }
        
        $defaultCorePath = $modx->getOption('core_path') . 'components/mySnippet/';
        $snippetCorePath = $modx->getOption('mySnippet.core_path', null, $defaultCorePath);
        $obj = $modx->getService('mySnippetClassMap', 'MyClass', $snippetCorePath . 'model/mySnippet/');
        if (!($obj instanceof MyClass))
            return '';
        
        $obj->initialize($modx->context->get('key'));
        
        /**
         * Submission posting into the database
         */
        if (!empty($hook) && !empty($_POST)) {
            $allFormFields = $hook->getValues();
        
            $report = $obj->submitForm($allFormFields);               // XXX This is the submission! XXX
        
            if ($report !== true) {
                $hook->addError('error_message', $report);
                return FALSE;
            } else {
                $modx->setPlaceholder('mySnippet.error.error_message', '');
                return TRUE;
            }
        }
        
        return '';
        

        Notice that submitForm method.

        Start my class
        <?php
        class MyClass {
            // ...
        

        The submitForm method:
        <?php
        
            /**
             * Sanitize then submit
             * @param   array   $entries    POST array
             * @return  bool    all valid and saved, or return FALSE
             */
            public function submitForm($entries) {
                $sanitizedEntries = $this->cleanInput($entries);       // Sanitize here
        
                $saveNewSubmit = $this->_saveNewSubmit ($sanitizedEntries); // This is the submission each by each.
                if ($saveNewSubmit !== TRUE) {
                    return $saveNewSubmit;
                }
                
                return TRUE;
            }
        


        The method below is the main sanitizer method.
        <?php
            /**
             * 1. Filter MODX's tag
             * 2. Internationalization
             * 3. Sanitizing user input from hacking
             * @param   array   $entries    raw input strings
             * @return  array   sanitized strings in an array
             */
            public function cleanInput($entries) {
                $entries = $this->_trimWhiteSpaces($entries);
                $entries = $this->utf8Rin($entries);
                $entries = $this->htmLawed($entries, null, array(
                            'safe' => 1,
                            'deny_attribute' => 'style',
                            'clean_ms_char' => 1,
                        ));
        
                return $entries;
            }
        


        The method below is only to strip extra spaces to be only one space.
        <?php
            /**
             * Trimming white spaces
             * @param   string  $source text to be trimmed
             * @return  string  trimmed text
             */
            private function _trimWhiteSpaces($source) {
                if (is_array($source)) {
                    foreach ($source as $k => $v) {
                        $source[$k] = $this->_trimWhiteSpaces($v);
                    }
                } else {
                    $source = trim($source);
                    $source = preg_replace("/\s\s+/", " ", $source);
                }
                return $source;
            }
        


        The method below is only about UTF8 characters.
        <?php
            /**
             *
             * Encoding using the class from
             * @author  Rin <http://forum.dklab.ru/profile.php?mode=viewprofile&u=3940>
             * @link    http://forum.dklab.ru/viewtopic.php?p=91015#91015
             * @param   mixed   $source         text to be converted
             * @param   string  $callback       call back function's name
             * @param   array   $callbackParams call back parameters (in an array)
             * @return  mixed   converted text
             */
            public function utf8Rin($source, $callback = null, $callbackParams = array()) {
                $path = $this->config['corePath'] . 'includes/utf8-rin/';
                include_once $path . 'ReflectionTypehint.php';
                include_once $path . 'UTF8.php';
        
                if ($callback === null) {
                    $callback = 'convert_to';
                }
        
                if (is_array($source)) {
                    foreach ($source as $k => $v) {
                        $source[$k] = $this->utf8Rin($source[$k], $callback, $callbackParams);
                    }
                } else {
                    if (empty($callbackParams)) {
                        $callbackParams = array(mb_detect_encoding($source));
                    }
        
                    $callbackParams = array_merge(array($source), $callbackParams);
                    $source = call_user_func_array(array('UTF8', $callback), $callbackParams);
                }
        
                return $source;
            }
        


        The method below is to strip HTML tags and such.
        <?php
            /**
             * Sanitizing string from 3rd party
             * @link    http://www.bioinformatics.org/phplabware/internal_utilities/htmLawed/
             * @param   mixed   $source         text to be converted
             * @param   string  $callback       call back function's name
             * @param   array   $callbackParams call back parameters (in an array)
             * @return  mixed   converted text
             */
            public function htmLawed($source, $callback=null, array $callbackParams = array()) {
                $path = $this->config['corePath'] . 'includes/htmLawed/';
                include_once $path . 'htmLawed.php';
        
                if ($callback === null) {
                    $callback = 'htmLawed';
                }
        
                if (is_array($source)) {
                    foreach ($source as $k => $v) {
                        $source[$k] = $this->htmLawed($source[$k], $callback, $callbackParams);
                    }
                } else {
                    $callbackParams = array_merge(array($source), $callbackParams);
                    $source = call_user_func_array($callback, $callbackParams);
                }
        
                return $source;
            }
        

        <?php
            // until end class
        }
        
          Rico
          Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
          MODx is great, but knowing how to use it well makes it perfect!

          www.virtudraft.com

          Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

          Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

          Maintainter/contributor of Babel

          Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
        • I would really caution about resetting the $_POST, $_REQUEST, and $_GET like you did in the examples 2,3,7.

          Resetting those could break other snippets/plugins. If any other extra relies on those (which is likely), that extra wont work.
            Chuck the Trukk
            ProWebscape.com :: Nashville-WebDesign.com
            - - - - - - - -
            What are TV's? Here's some info below.
            http://modxcms.com/forums/index.php/topic,21081.msg159009.html#msg1590091
            http://modxcms.com/forums/index.php/topic,14957.msg97008.html#msg97008
          • Well, actually they are about filtering which characters are allowed for further process.
            Returning back the $_POST after sanitizing also can be done, if I’d like to keep the global $_POST variable still available.

            <?php //highlight
            $sanitizedPosts = $modx->sanitize($_POST);
            $_POST = array();
            $_POST = sanitizedPosts;                      // also can be done
            


            My habit is just not using that ’$_POST’ variable again to remind me that since that time I deal with the $sanitizedPost.
            I know that some snippets wrap other snippets like getPage->getResources.
            Well, recursive cleaning is not a sin. tongue
              Rico
              Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
              MODx is great, but knowing how to use it well makes it perfect!

              www.virtudraft.com

              Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

              Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

              Maintainter/contributor of Babel

              Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
            • Everyone knows that the GPC variables can already be sanitized on any request that goes through the MODX front-end gateway (i.e. index.php), right? You do not necessarily need to apply this manually (see NOTE below). In addition, if you use the MODX objects/API to make changes to the database, that will also be automatically quoted via PDO prepared statement bindings, e.g.
              <?php
              $myObject = $modx->getObject('myClass', $myPK);
              if ($myObject) {
                  /* fromArray() calls set() for each $_POST var that matches a field in your object */
                  $myObject->fromArray($_POST);
                  /* save() uses prepared statement bindings to automatically quote the values being sent to the db */
                  $myObject->save();
              }
              

              But remember, this is just basic sanitization—it is still up to you to define what is and what isn’t valid user input based on specific usage requirements. What if you need to allow some HTML tags? There is a gray area between security and validation here, but a solid combination of both, along with making sure anything that is output back to the user contains nothing that can be exploited (e.g. XSS injection) should always be the goal.

              NOTE: There is a System Setting, allow_tags_in_post to allow MODX and HTML tags in the $_POST variable which is enabled by default. To not allow this, for instance in the front-end of your website, you can add this as a Context Setting set to "0" to the web (or any other front-end) Context. This is only enabled by default so that both back-end and front-end content editing solutions will work.
              • @OpenGeek,
                That’s for the $_POST.
                How about the $_GET (let’s say, for the AJAX connector) ?
                  Rico
                  Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
                  MODx is great, but knowing how to use it well makes it perfect!

                  www.virtudraft.com

                  Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

                  Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

                  Maintainter/contributor of Babel

                  Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
                • More tips on using $_GET:

                  If you use $_GET in your scripts,

                  • try to encode it (eg: base64_encode) + use hash if you need to.
                  • Salting the value is also very useful, especially if the Salt is not a static value, but a random string that is stored inside the database, which is different to each installed site.

                  $_GET is a common hole for the hacker to break your site.

                  That’s also applied to the Ajax processor/controller files.
                    Rico
                    Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
                    MODx is great, but knowing how to use it well makes it perfect!

                    www.virtudraft.com

                    Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

                    Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

                    Maintainter/contributor of Babel

                    Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
                  • Quote from: goldsky at Apr 05, 2011, 02:22 PM

                    <?php //highlight
                    $sanitizedPosts = $modx->sanitize($_POST);
                    $_POST = array();
                    $_POST = sanitizedPosts;                      // also can be done
                    


                    Somewhat new to PHP security issues here. I am building on the Authorize.net payment form for Visioncart, and I want to make sure my code is secure against any kind of script injection badness etc. The credit card form is processed and validated through Formit. Should simply doing recursive sanitizing on $_POST as in this example, called as a preHook, give me a pretty good wall of protection in principle? I realize this question may be too general to answer and I can post specific code if that would help.
                      Jason
                    • That’s depends on the whole script, though.

                      If it’s placed on the webroot, you should check the access validation to the direct file.
                      Especially if you’re using any AJAX processor file.

                      If you’re using 3rd party classes, check them too.
                        Rico
                        Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
                        MODx is great, but knowing how to use it well makes it perfect!

                        www.virtudraft.com

                        Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

                        Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

                        Maintainter/contributor of Babel

                        Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.