Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-60940

Add ability to force cleaning all user texts

    XMLWordPrintable

    Details

    • Testing Instructions:
      Hide
      • Leave the config flag "forceclean" (site admin -> security -> site policies) in the default disabled state.
      • Submit raw HTML content with <script> and <iframe> tags to some areas that are not normally cleaned - such as course section descriptions or HTML block on the user's dashboard page
      • TEST: Check that this raw HTML is not cleaned when displayed: JS is executed, iframe is displayed.
      • Turn "forceclean" (site admin -> security -> site policies) setting on.
      • TEST: Check that editing a content that is normally cleaned anyway (such as forum post).
      • TEST: Check that the content is cleaned now when displayed: JS not executed, iframe not displayed.
      Show
      Leave the config flag "forceclean" (site admin -> security -> site policies) in the default disabled state. Submit raw HTML content with <script> and <iframe> tags to some areas that are not normally cleaned - such as course section descriptions or HTML block on the user's dashboard page TEST: Check that this raw HTML is not cleaned when displayed: JS is executed, iframe is displayed. Turn "forceclean" (site admin -> security -> site policies) setting on. TEST: Check that editing a content that is normally cleaned anyway (such as forum post). TEST: Check that the content is cleaned now when displayed: JS not executed, iframe not displayed.
    • Affected Branches:
      MOODLE_34_STABLE
    • Fixed Branches:
      MOODLE_35_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-60940-master-forceclean

      Description

      This was originally discussed at https://moodle.org/local/chatlogs/index.php?conversationid=21326 and that chat provide more related information.

      A friend of mine Luděk Šulák (moodledev at a Moodle partner) raised an interesting idea - ability to forbid the "noclean" flag functionality. So even when it is hard-coded that the content should not be cleaned (such as mod_page contents, course section descriptions or HTML block on user's own dashboard page etc), it still would be. The reasoning behind is that 99% of the course creators do not need to attach JS to the HTML. So just to support very rare use-cases, we leave quite a big opportunity for attackers.

      This will come with a cost of loosing certain features - such as unable to include custom iframes (as was raised in the chat). Still, some people see the current behaviour as a real security issue - for example MDL-50326. The reasoning has been that "some users somewhere maybe knows and wants to use this" so we force the rest of the world to swallow it and accept that we do not clean the HTML contents in certain places. But with the same logic applied, "some admin somewhere" wants to turn this off, and we do not give them an easy option.

      This is a proposal to introduce a new $CFG->forceclean flag that would make the noclean flag ignored and would make clean_text() be always applied.

      This must be communicated clearly with admins so they do not false into a trap of false security feeling. There are other ways how to bypass this. But it makes it harder.

        Attachments

          Issue Links

            Activity

              People

              • Votes:
                2 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  17/May/18