We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 20413
    • 2,877 Posts
    They’re unfortunate initials, I know...

    They totally ROCK!!!!!
      @hawproductions | http://mrhaw.com/

      Infograph: MODX Advanced Install in 7 steps:
      http://forums.modx.com/thread/96954/infograph-modx-advanced-install-in-7-steps

      Recap: Portland, OR (PDX) MODX CMS Meetup, Oct 6, 2015. US Bancorp Tower
      http://mrhaw.com/modx_portland_oregon_pdx_modx_cms_meetup_oct_2015_us_bancorp_tower
      • 28042 ☆ A M B ☆
      • 24,524 Posts
      Can’t be any worse than FCK grin
        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
        • 3749
        • 24,544 Posts
        Does anybody feel like testing SPForm for XSS vulnerability? It does check all the inputs so I’d like to think it’s safe, but I don’t have time to test it right now and I’d hate to think that it’s putting sites at risk.

          Did I help you? Buy me a beer
          Get my Book: MODX:The Official Guide
          MODX info for everyone: http://bobsguides.com/modx.html
          My MODX Extras
          Bob's Guides is now hosted at A2 MODX Hosting
          • 13226
          • 953 Posts
          PMS - your a star

          I havent posted in the last couple of days as I have been upgrading all of my sites and researching XSS further.

          As I was scouring the net yesterday I came across: "function RemoveXSS" - http://kallahar.com/smallprojects/php_xss_filter_function.php and thought it might be somethig that could be adapted for eForm - obviously i’m not a programmer and have no idea if this code can be adapted at all.

          In case the page isnt reachable I have attached the code as "function-RemoveXSS.txt"

          If it cant be adapted then at least the initial idea could possibly be implemented with different coding.

          According to the developer the code is "Public Domain"

          License:
          This code is public domain, you are free to do whatever you want with it, including adding it to your own project which can be under any license.

          I have also created a possible "Badwords" list (see attachment - badwords.txt)
            • 13226
            • 953 Posts
            This may seem "over the top" - but security comes first wink

            Now that "PMS" has modified eForm with "safestring" would it not be beneficial to also add the JOT "Badwords" function to the core eForm code ?

            If I understand correctly the "safestring" checks if HTML characters are in any input areas and removes them. So if you have set eForm to accept HTML characters then "safestring" cant be used.

            So adding the badwords function you could use "string" and check for individual HTML characters and have them removed.

            The following has been copied from "jot.class.inc.php".
            // Badwords
            $this->config["badwords"]["enabled"] = !is_null($this->Get("badwords")) ? 1 : 0;
            $this->config["badwords"]["type"] = !is_null($this->Get("bw")) ? intval($this->Get("bw")) : 1;
            if($this->config["badwords"]["enabled"]) {
            	$badwords = $this->Get("badwords");
            	$badwords = preg_replace("~([\n\r\t\s]+)~","",$badwords);
            	$this->config["badwords"]["words"] = explode(",",$badwords);
            	$this->config["badwords"]["regexp"] = "~" . implode("|",$this->config["badwords"]["words"]) . "~i";
            }
            


            The Eform badwords parameter could then be the same as the JOT parameter(s).
            [!eForm? &badwords=`{{Badwords}}` &formid=`Test Form` &to=`{{my_mailaddress_chunk}}` &tpl=`testform` &report=`testform_report` &mailselector=`department` &vericode=`1` &sendAsHtml=`0` &allowhtml=`0` &subject=`[+subject+]`!]
            


            With the addition of my previous post a modified version of the "RemoveXSS Function" could help eForm to become a well protected piece of code.

            To sum up:

            eForm could have a combination or all of the following:

            • Safestring - removing all HTML entities from all input fields
            • Badwords - removing HTML entities and typical spam and/or general rubbish
            • RemoveXSS - help stop any or almost all XSS attacks

            It would be nice to here what others have to say about this idea grin
              • 22851
              • 805 Posts
              I think we need a saferhtml format. I say saferhtml and not safehtml - because it must be appreciated that getting a very safe html accepting format will be difficult. A saferhtml format would ensure that none of these were in the final output:

              - php tags (of any type)
              - html/xml comments
              - any form based tags (input, button, select, ...)
              - script tags (<script> and legacy <javascript>)
              - object, iframe, applet tags
              - javascript events
              - unescaped html entities within the text content
              - style attributes or embedded style

              and would have to ensure that any elements that accept URLs only accept valid ones using the http or https protocols.
              - a
              - img
              - cite
              etc.

              This is how I suggest it should be done. First, use strip_tags to remove php tags, xml/html comments etc. - but specify a whitelist of tags that we want to keep.

              Here’s the full list from w3schools. I have deleted deprecated syntax and elements we should probably exclude from the whitelist.

              <a><abbr><acronym><address><applet><area><b><base><basefont>
              <bdo><big><blockquote><body>
              <button><caption><center><cite><code><col><colgroup><dd><del><dfn><dir><div><dl><dt><em><fieldset><font><form><frame><frameset><head><h1> - <h6><hr><html><i><iframe><img><input><ins><isindex><kbd><label><legend><li><link><map><menu><meta><noframes><noscript><object><ol><optgroup><option><p><param><pre><q><s><samp><script><select><small><span><strike><strong><style><sub><sup><table><tbody><td><textarea><tfoot><th><thead><title><tr><tt><u><ul><var><xmp>

              Then, pass it through an html validator like htmlLawed. There are probably plenty of others to choose from. Perhaps we should do some more investigation.

              That shouldn’t be too difficult to implement and should be a huge improvement on what we currently have. I wouldn’t bother including a badwords list as part of saferhtml. A badwords list doesn’t make it any safer in my opinion - it just provides an option to exclude posts that contain certain words you don’t like.
                YAMS: Yet Another Multilingual Solution for MODx
                YAMS Forums | Latest: YAMS 1.1.9 | YAMS Documentation
                Please consider donating if you appreciate the time and effort spent developing and supporting YAMS.
                • 24935
                • 160 Posts
                I agree with a whitelist approach rather than a blacklist--a whitelist is the only real way to be safe. However, there needs to ba a whitelist of html tags AND an whitelist of attributes to eliminate all of the onload, and oneverythingelse attributes, etc. while allowing some good attributes like href, rel, src, alt, title

                There are some some functions called strip_tags_attributes that should be able to do this
                http://us2.php.net/manual/en/function.strip-tags.php (in the comments)
                http://www.phpinform.com/2009/07/16/php-function-that-strips-both-tags-and-atributes/

                What would be the best would be a default whitelist of tags and attributes that can be overridden.
                  • 22851
                  • 805 Posts
                  strip_tags wont strip attributes on tags which are in the whitelist, like you say. I’m a bit nervous about using those functions in the php documentation comments as is to strip attributes - since they a are neither fully tested nor approved.


                  However, there needs to ba a whitelist of html tags AND an whitelist of attributes to eliminate all of the onload, and oneverythingelse attributes, etc. while allowing some good attributes like href, rel, src, alt, title

                  htmlLawed claims to do all those things, and even has an anti-XSS test page and notes on tinyMCE integration.
                    YAMS: Yet Another Multilingual Solution for MODx
                    YAMS Forums | Latest: YAMS 1.1.9 | YAMS Documentation
                    Please consider donating if you appreciate the time and effort spent developing and supporting YAMS.
                    • 21246
                    • 55 Posts
                    What is the status of the XSS vulnerability? I have not dived into this yet and would like to determine the resolution. @PMS, did you do your testing against 1.4.4.6 included in Evo?
                      • 22851
                      • 805 Posts
                      Quote from: netdoctor at Sep 07, 2009, 03:18 PM

                      What is the status of the XSS vulnerability? I have not dived into this yet and would like to determine the resolution. @PMS, did you do your testing against 1.4.4.6 included in Evo?
                      The files in this post which upgrade eForm from version 1.4.4.6 to a potential version 1.4.4.7 work with eForm running on Evolution. This new version introduces a new datatype called safestring which has been set as the default. This new datatype will strip all code from the input and escape it for html output. This acts as a pretty good barrier against XSS attempts compared to the old default datatype, string, which was completely unsafe for inclusion in a webpage or email without further processing.

                      Note however, that if the html datatype is used then the posted info is basically output unchecked by default - and so it is unsafe to use this without further validation. To do so would leave a site open to XSS attacks. In this post I put forward a proposal for a new datatype called saferhtml that would allow html tag and attribute whitelists and validation using htmlLawed... but I got bogged down in other things (YAMS) and never got around to implementing anything. If anyone wants to pick up the baton, feel free.
                        YAMS: Yet Another Multilingual Solution for MODx
                        YAMS Forums | Latest: YAMS 1.1.9 | YAMS Documentation
                        Please consider donating if you appreciate the time and effort spent developing and supporting YAMS.