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

lib_form/filemanager: erroneous escaping on reference list

    Details

    • Testing Instructions:
      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.
    • Affected Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-45387-master

      Description

      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 Andrew Nicols, 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.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                • Votes:
                  0 Vote for this issue
                  Watchers:
                  6 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    12/May/14