Moodle
  1. Moodle
  2. MDL-30496

Incorrect checking of upload server limit in WebService upload.php script

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1, 2.2
    • Fix Version/s: 2.2
    • Component/s: Web Services
    • Labels:
    • Rank:
      33188

      Description

      If you set Administration > Security -> Site policies -> Maximum uploaded file size to sever limit the $CFG>maxbytes setting will be set to 0, this will prevent any file to be uploaded

      Check:

      if (($_FILES[$fieldname]['size'] > $CFG->maxbytes)) {
      

      This will always return true, so you can't upload files

      I think this will fix the problem:

      if ($CFG->maxbytes && ($_FILES[$fieldname]['size'] > $CFG->maxbytes)) {
      

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks again.

          Show
          Michael de Raadt added a comment - Thanks again.
          Hide
          Jérôme Mouneyrac added a comment - - edited

          Hi Apu, can you cherry-pick: https://github.com/mouneyrac/moodle/commit/00bad4ba6d858b3a9aa1765d85870d500544be7a

          I added a second commit this morning to MDL-30459. Since it has been integrated I added the above third commit to it. So in fact you just need to cherry-pick this third commit.
          The reason I added this third commit is that it fixes an inconsistence introduced in the second commit. Is that clear ?

          Show
          Jérôme Mouneyrac added a comment - - edited Hi Apu, can you cherry-pick: https://github.com/mouneyrac/moodle/commit/00bad4ba6d858b3a9aa1765d85870d500544be7a I added a second commit this morning to MDL-30459 . Since it has been integrated I added the above third commit to it. So in fact you just need to cherry-pick this third commit. The reason I added this third commit is that it fixes an inconsistence introduced in the second commit. Is that clear ?
          Hide
          Jérôme Mouneyrac added a comment - - edited

          I've just noticed quite some differences between 2.1 and 2.2. I lost track of backporting changes after MDL-29036. Apu can you just integrate this into master. I created an issue for backporting missing changes in 2.1 (MDL-30542). Thanks.

          Note: in 2.0 we don't have any upload script.

          Show
          Jérôme Mouneyrac added a comment - - edited I've just noticed quite some differences between 2.1 and 2.2. I lost track of backporting changes after MDL-29036 . Apu can you just integrate this into master. I created an issue for backporting missing changes in 2.1 ( MDL-30542 ). Thanks. Note: in 2.0 we don't have any upload script.
          Hide
          Aparup Banerjee added a comment -

          bringing this is for integration - needs to be fixed.

          Show
          Aparup Banerjee added a comment - bringing this is for integration - needs to be fixed.
          Hide
          Aparup Banerjee added a comment -

          Thanks, this (3b40f3b) has been integrated into master. up for testing.

          Show
          Aparup Banerjee added a comment - Thanks, this (3b40f3b) has been integrated into master. up for testing.
          Hide
          Rossiani Wijaya added a comment -

          This is working great.

          Thanks Jerome for fixing it.

          Test passed.

          Show
          Rossiani Wijaya added a comment - This is working great. Thanks Jerome for fixing it. Test passed.
          Hide
          Jérôme Mouneyrac added a comment -

          No worries. Thanks to Juan and Patrick for reporting and also for the fix. Cheers.

          Show
          Jérôme Mouneyrac added a comment - No worries. Thanks to Juan and Patrick for reporting and also for the fix. Cheers.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Sent upstream! Just in time for Moodle 2.2rc1 (if related), yay!

          Closing and big thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Sent upstream! Just in time for Moodle 2.2rc1 (if related), yay! Closing and big thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: