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

Missing htmlentities() call when serving a stored_file

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.1.4, 3.2.1, 3.3
    • Fix Version/s: 3.3
    • Component/s: Files API
    • Labels:
    • Affected Branches:
      MOODLE_31_STABLE, MOODLE_32_STABLE, MOODLE_33_STABLE
    • Fixed Branches:
      MOODLE_33_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      m33_MDL-58027_Restore_Missing_HTMLEntities_Call
    • Testing Instructions:
      Hide

      <Not sure how to trigger the issue w/o hacking the code />

      1. Tests should run fine (no human action required here, CI will tell us in 24h).
      2. Files should be served regularly: perform a quick smock test using some modules and resources
      Show
      <Not sure how to trigger the issue w/o hacking the code /> Tests should run fine (no human action required here, CI will tell us in 24h). Files should be served regularly: perform a quick smock test using some modules and resources

      Description

      While refactoring send_stored_file() in MDL-57789, a missing htmlentities() has been found: https://github.com/scara/moodle/blob/9ec952f237d74cb161217045a40c2ae1d56a30bf/lib/filelib.php#L2428.

      No one has complained about this missing so far, given that none of the modules should apply filters while serving a file, e.g. SCORM: https://moodle.org/mod/forum/discuss.php?d=167328#p810497. More examples:

      # grep -R "send_stored_file(\$file, \$lifetime," *
      admin/lib.php:            send_stored_file($file, $lifetime, 0, false, $options);
      lib/outputlib.php:            send_stored_file($file, $lifetime, 0, $forcedownload, $options);
      mod/book/lib.php:        send_stored_file($file, $lifetime, 0, $forcedownload, $options);
      mod/scorm/lib.php:    send_stored_file($file, $lifetime, 0, false, $options);
      repository/filesystem/lib.php:    send_stored_file($file, $lifetime, 0, $forcedownload, $options);
      

      The code potentially affected by serving (plain text) files w/o the missing call looks like:

      file.php:send_stored_file($file, null, $CFG->filteruploadedfiles, $forcedownload);
      mod/resource/lib.php:    send_stored_file($file, null, $filter, $forcedownload, $options);
      repository/dropbox/lib.php:            send_stored_file($storedfile, $lifetime, $filter, $forcedownload, $options);
      repository/lib.php:                send_stored_file($srcfile, $lifetime, $filter, $forcedownload, $options);
      

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              matteo Matteo Scaramuccia
              Reporter:
              matteo Matteo Scaramuccia
              Peer reviewer:
              Juan Leyva Juan Leyva
              Integrator:
              Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
              Tester:
              Eloy Lafuente (stronk7) Eloy Lafuente (stronk7)
              Participants:
              Component watchers:
              Matteo Scaramuccia, Andrew Lyons, Dongsheng Cai, Huong Nguyen, Jun Pataleta, Michael Hawkins, Shamim Rezaie, Simey Lameze
              Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                15/May/17