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

mod_data: Missing validation of image width and height

    XMLWordPrintable

Details

    • MOODLE_400_STABLE, MOODLE_401_STABLE
    • MOODLE_400_STABLE, MOODLE_401_STABLE
    • MDL-76525-MOODLE_400_STABLE
    • MDL-76525-MOODLE_401_STABLE
    • MDL-76525-master
    • Hide
      Requirements
      • Server running on PHP 7.4 but can be switched to run PHP 8.0
      Before the patch
      1. Use PHP 7.4
      2. As teacher, create database activity in a course
      3. Add a new field of type "Image"
      4. Insert a name for the field
      5. Insert a non-numeric value for the following fields (e.g. "214px", etc.):
        • Width (in pixels) in single view
        • Height (in pixels) in single view
        • Width (in pixels) in list view
        • Height (in pixels) in list view
      6. Click "Save"
      Test: After the patch
      1. Switch to PHP 8.0
      2. Add an entry for database, setting a picture for the Image field you created earier.
      3. Save the entry.
      4. Confirm that you don't get any error
      5. Confirm that the image you entered is rendered in both List and Single views.
      6. Edit the image field.
      7. Confirm that the non-numeric width/height values are now empty.
      8. Confirm that you can only enter numeric values (e.g. numbers, decimal point)
      9. Save the form.
      10. Apply the test patch to revert the width/height text fields to accept non-numeric inputs.

        git apply MDL-76525-test.diff 
        

      11. Enter non-numeric values for the width/height inputs.
      12. Save the changes.
      13. Confirm that the page is reloaded with an error notification listing the fields that have invalid values.
      14. Create a new image field
      15. Enter non-numeric values for the width/height inputs.
      16. Save the changes.
      17. Confirm that the page is reloaded with an error notification listing the fields that have invalid values. (You'll notice that the inputs are blank again when the page reloads. This is okay).
      After test
      • Make sure to revert the changes from MDL-76525-test.diff

        git reset --hard 
        

      Show
      Requirements Server running on PHP 7.4 but can be switched to run PHP 8.0 Before the patch Use PHP 7.4 As teacher, create database activity in a course Add a new field of type "Image" Insert a name for the field Insert a non-numeric value for the following fields (e.g. "214px", etc.): Width (in pixels) in single view Height (in pixels) in single view Width (in pixels) in list view Height (in pixels) in list view Click "Save" Test: After the patch Switch to PHP 8.0 Add an entry for database, setting a picture for the Image field you created earier. Save the entry. Confirm that you don't get any error Confirm that the image you entered is rendered in both List and Single views. Edit the image field. Confirm that the non-numeric width/height values are now empty. Confirm that you can only enter numeric values (e.g. numbers, decimal point) Save the form. Apply the test patch to revert the width/height text fields to accept non-numeric inputs. git apply MDL-76525- test . diff Enter non-numeric values for the width/height inputs. Save the changes. Confirm that the page is reloaded with an error notification listing the fields that have invalid values. Create a new image field Enter non-numeric values for the width/height inputs. Save the changes. Confirm that the page is reloaded with an error notification listing the fields that have invalid values. (You'll notice that the inputs are blank again when the page reloads. This is okay). After test Make sure to revert the changes from MDL-76525 -test.diff git reset --hard
    • HQ Sprint 1.3 Moppies

    Description

      If you add an image field in the database it is possible to specify a width and height for the single and list view.

      However, these fields are not being validated properly. It is possible to insert strings, for example "300px" which lead to an error as soon as a user tries to add an entry.

      To reproduce:

      • Create database activity
      • Define a new field "Image"
      • Specify "Width (in pixels) in list view" or "Height (in pixels) in list view" and set one of them to "300px". Save.
      • Add a database entry: Upload an image. Save.
      • With debug mode on, you will see a notice about a malformed string in PHP 7.4.
      • In PHP 8.0 (and probably above) you will receive an error (also without debug mode enabled), see screenshot. This will fail the adding of the entry!

      Suggested solution: Add proper validation to these fields.
      Additional info: This does not occur for "Width (in pixels) in single view" and "Height (in pixels) in single view" for some reason.

      Attachments

        1. 400_after_patch_with diff.gif
          400_after_patch_with diff.gif
          504 kB
        2. 400_after_patch.gif
          400_after_patch.gif
          292 kB
        3. 400_before Patch.gif
          400_before Patch.gif
          105 kB
        4. 401_after_patch_with diff.gif
          401_after_patch_with diff.gif
          319 kB
        5. 401_after_patch.gif
          401_after_patch.gif
          426 kB
        6. 401_before patch.gif
          401_before patch.gif
          159 kB
        7. Error when adding a new entry including an image.png
          Error when adding a new entry including an image.png
          72 kB
        8. expected_behaviour.gif
          expected_behaviour.gif
          303 kB
        9. image-2022-12-13-07-14-23-917.png
          image-2022-12-13-07-14-23-917.png
          46 kB
        10. image-2022-12-13-07-14-43-094.png
          image-2022-12-13-07-14-43-094.png
          35 kB
        11. master_after_patch_with diff.gif
          master_after_patch_with diff.gif
          451 kB
        12. master_after_patch.gif
          master_after_patch.gif
          473 kB
        13. master_before_patch.gif
          master_before_patch.gif
          133 kB
        14. MDL-76525-test.diff
          2 kB
        15. messages_displayed_after_saving_creation.gif
          messages_displayed_after_saving_creation.gif
          935 kB
        16. messages_displayed_after_saving_edition.gif
          messages_displayed_after_saving_edition.gif
          244 kB

        Issue Links

          Activity

            People

              phmemmel PhMemmel
              phmemmel PhMemmel
              Laurent David Laurent David
              Jun Pataleta Jun Pataleta
              Ron Carl Alfon Yu Ron Carl Alfon Yu
              Amaia Anabitarte, Carlos Escobedo, Laurent David, Mikel Martín Corrales, Sabina Abellan, Sara Arjona (@sarjona)
              Votes:
              7 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                13/Mar/23

                Time Tracking

                  Estimated:
                  Original Estimate - 0 minutes
                  0m
                  Remaining:
                  Remaining Estimate - 0 minutes
                  0m
                  Logged:
                  Time Spent - 6 hours, 40 minutes
                  6h 40m