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

Cache admin page is using sloppy PARAM_TEXT when it should be PARAM_SAFEPATH

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.4.2
    • Component/s: Caching
    • Labels:
    • Testing Instructions:
      1. Log in as an admin
      2. Browse to Settings > Plugins > Caching > Configuration
      3. Edit the mappings for a definition and save your changes.
      4. Make sure you get no errors.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE

      Description

      As identified in MDL-37474 the cache admin page is using PARAM_TEXT for cache definitions. It should definitely be using PARAM_ALPHANUMEXT instead.

      MDL-37474 saw this addressed in master, this issue is about backporting that small change to 24.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              samhemelryk Sam Hemelryk added a comment -

              2.4 only thanks integrator

              Show
              samhemelryk Sam Hemelryk added a comment - 2.4 only thanks integrator
              Hide
              damyon Damyon Wiese added a comment -

              Re: comments on linked issue - I don't think this is a security issue - I don't see any way it could be exploited.

              Show
              damyon Damyon Wiese added a comment - Re: comments on linked issue - I don't think this is a security issue - I don't see any way it could be exploited.
              Hide
              damyon Damyon Wiese added a comment -

              Hi Sam,

              This change is not correct - definition gets split into:

              list($component, $area) = explode('/', $definition, 2);

              Changing to PARAM_ALPHANUMEXT causes the '/' to be stripped - I get a coding_error on the view the mappings page for any cache definition with this patch.

              • Damyon
              Show
              damyon Damyon Wiese added a comment - Hi Sam, This change is not correct - definition gets split into: list($component, $area) = explode('/', $definition, 2); Changing to PARAM_ALPHANUMEXT causes the '/' to be stripped - I get a coding_error on the view the mappings page for any cache definition with this patch. Damyon
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks for picking that up Damyon, I've fixed it up and once more it is up for integration.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks for picking that up Damyon, I've fixed it up and once more it is up for integration.
              Hide
              damyon Damyon Wiese added a comment -

              Thanks Sam - this has been integrated now.

              Show
              damyon Damyon Wiese added a comment - Thanks Sam - this has been integrated now.
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks for fixing this Sam,

              No error found while editing and saving mapping.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks for fixing this Sam, No error found while editing and saving mapping.
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

              (and won't be revisiting it unless some regression is found)

              Thanks and ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    11/Mar/13