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

lib_form/filemanager: erroneous escaping on reference list

XMLWordPrintable

    • MOODLE_25_STABLE, MOODLE_26_STABLE
    • MOODLE_25_STABLE, MOODLE_26_STABLE
    • MDL-45387-master
    • Hide

      This test should be performed on 2.5, 2.6 and master; there shouldn't be a need to do a cross-browser test.

      1. Before applying the patch, upload an image to your private files area.
      2. In a new course:
        • Create a new page, and insert the image from step 2 as an alias/shortcut.
        • Repeat with another page.
      3. Go to your private files area.
        • Observe that your image now has a 'link' icon next to it signifying that it has associated aliases/shortcuts.
      4. Click your image icon to bring up the editing dialog.
        • Observe that the list of aliases/shortcuts does not appear as it should; li tags appear in the escaped output.
      5. Apply the patch, and refresh the page.
        • Observe that the list of aliases/shortcuts now appears as it should.
      Show
      This test should be performed on 2.5, 2.6 and master; there shouldn't be a need to do a cross-browser test. Before applying the patch, upload an image to your private files area. In a new course: Create a new page, and insert the image from step 2 as an alias/shortcut . Repeat with another page. Go to your private files area. Observe that your image now has a 'link' icon next to it signifying that it has associated aliases/shortcuts. Click your image icon to bring up the editing dialog. Observe that the list of aliases/shortcuts does not appear as it should; li tags appear in the escaped output. Apply the patch, and refresh the page. Observe that the list of aliases/shortcuts now appears as it should.

      Discovered while testing MDLQA-7051. When a file has aliases/shortcuts present in a course, and a user uses the file manager to edit the file's settings, the list of aliases/shortcuts is erroneously escaped.

      What should happen
      Each li element's contents should be escaped.

      What actually happens
      The entire contents of the ul, including the li tags, are escaped - resulting in http://puu.sh/8vLVh/70c902163c.png

      The offending line of Javascript is here, introduced with MDL-37507 (security issue): https://github.com/moodle/moodle/blame/MOODLE_25_STABLE/lib/form/filemanager.js#L1022

      After consulting with dobedobedoh, a way to fix this has been identified as instead of performing a Y.Escape on the entire contents as above, we should instead perform a Y.Escape in the preceding for loop, i.e.

      for (var i in obj.references) {
      		 
          node.reflist += '<li>'+Y.Escape.html(obj.references[i])+'</li>';
      		 
      }
      		 
      selectnode.one('.fp-reflist .fp-value').setContent(node.reflist);

      Testing should encompass a re-test of MDL-37507 just in case to avoid any regressions.

        1. MDL-45387_25.patch
          1 kB
        2. MDL-45387_26.patch
          1 kB
        3. MDL-45387_master.patch
          1 kB
        4. Unescaped list.jpg
          Unescaped list.jpg
          49 kB

            jethac Jetha Chan
            jethac Jetha Chan
            Frédéric Massart Frédéric Massart
            Marina Glancy Marina Glancy
            Sam Hemelryk Sam Hemelryk
            Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.