[MDL-69129] Admin config changes report SQL ORDER BY not limited to ASC/DESC strings - Moodle Tracker

XML

Word

Printable

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 3.5.12, 3.6.10, 3.7.6, 3.8.3
Fix Version/s: 3.8.4
Component/s: Reports
Labels:

Affected Branches:
MOODLE_35_STABLE, MOODLE_36_STABLE, MOODLE_37_STABLE, MOODLE_38_STABLE
Fixed Branches:
MOODLE_38_STABLE
Testing Instructions:
Hide

Log in as admin on a site where some config options have previously been changed.

Visit the following:

<wwwroot>/report/configlog/index.php?sort=name&dir=DESC&page=0&perpage=30

CONFIRM the rows are sorted by the Setting column in reverse alphabetical order (ie z-a, so something like 'youtube' would be towards the top of the table).

CONFIRM the icon next to the Setting heading is an up arrow.

Click on the "Setting" column heading.

CONFIRM the rows are still sorted by the Setting column, but are now sorted in alphabetical order (ie a-z, so something earlier alphabetically will appear at the top)

CONFIRM the icon next to the Setting heading is a down arrow.

In the address bar of your browser, change "dir=ASC" to "dir=ASCCCCCC" (ie an invalid value) and press enter.

CONFIRM the rows are now sorted in reverse alphabetical order again, with the up arrow icon next to the Setting heading.
Show
Log in as admin on a site where some config options have previously been changed. Visit the following: <wwwroot>/report/configlog/index.php?sort=name&dir=DESC&page=0&perpage=30 CONFIRM the rows are sorted by the Setting column in reverse alphabetical order (ie z-a, so something like 'youtube' would be towards the top of the table). CONFIRM the icon next to the Setting heading is an up arrow. Click on the "Setting" column heading. CONFIRM the rows are still sorted by the Setting column, but are now sorted in alphabetical order (ie a-z, so something earlier alphabetically will appear at the top) CONFIRM the icon next to the Setting heading is a down arrow. In the address bar of your browser, change "dir=ASC" to "dir=ASCCCCCC" (ie an invalid value) and press enter. CONFIRM the rows are now sorted in reverse alphabetical order again, with the up arrow icon next to the Setting heading.

Sprint:
International 4.0 - Sprint 1

Description

The admin config changes report (Site admin > Reports > Config changes) accepts a "dir" GET parameter to determine whether to order the report by ASC or DESC. The problem with its implementation is that it injects an alpha filtered version of that value directly into the SQL, instead of using logical operators to insert a predefined string of ASC or DESC.

Since it does only allow alphabet characers (along with its position in the query), I don't think its susceptible to malicious SQL injection, the result will just be that the query will be rendered invalid and cause an error on the page. I have, however, set this to "could be a security issue" for the time being, pending whoever takes the issue doing some further investigation.

Note: The "dir" parameter has been removed from Moodle 3.9, so that version and later are not affected.

(Prepared by michaelh based on report by Spyridon Chatzimichail, SF case 00076304)

Attachments

- Sort By Name
- Sort By Date
- Ascending
- Descending
- Thumbnails
- List
- Download All

MDL-69129.jpg
47 kB
08/Jul/20 4:08 PM
MDL-69129-35.mdk.patch
0.8 kB
26/Jun/20 6:26 PM
MDL-69129-37.mdk.patch
0.8 kB
26/Jun/20 6:26 PM
MDL-69129-38.mdk.patch
0.8 kB
26/Jun/20 6:22 PM

Activity

People

Assignee:: Michael Hawkins

Reporter:: Spyridon Chatzimichail

Peer reviewer:: Mathew May

Integrator:: Andrew Lyons

Tester:: Anna Carissa Sadia

Participants:: Andrew Lyons, Anna Carissa Sadia, CiBoT, Mathew May, Michael Hawkins, Spyridon Chatzimichail

Component watchers:

Votes:: 0 Vote for this issue

Watchers:: 4 Start watching this issue

Dates

Created:: 24/Jun/20 5:26 PM

Updated:: 11/Jul/20 2:48 AM

Resolved:: 11/Jul/20 2:48 AM

Fix Release Date:: 13/Jul/20

Time Tracking

Estimated:

Not Specified

Remaining:

Logged:

5h 30m