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:

      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:

      https://github.com/moodle/moodle/blob/master/webservice/upload.php#L111

      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)) {

        Gliffy Diagrams

          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: