Moodle
  1. Moodle
  2. MDL-37747

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.4.2
    • Component/s: Caching
    • Labels:
    • Rank:
      47466

      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.

        Issue Links

          Activity

          Hide
          Sam Hemelryk added a comment -

          2.4 only thanks integrator

          Show
          Sam Hemelryk added a comment - 2.4 only thanks integrator
          Hide
          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 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 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 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
          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
          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 Wiese added a comment -

          Thanks Sam - this has been integrated now.

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

          Thanks for fixing this Sam,

          No error found while editing and saving mapping.

          Show
          Rajesh Taneja added a comment - Thanks for fixing this Sam, No error found while editing and saving mapping.
          Hide
          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
          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: