Non-core contributed modules

Confirm data manipulations before execution

Details

  • Affected Branches:
    MOODLE_19_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE

Description

Tim - Good work on the custom sql admin report. I was thinking that the Partners may find this helpful as an alternative to PHPMyAdmin (as mentioned in the forums). One thing that I think would improve things would be to require confirmation before defining (not necessarily for each execution) if there is risk of it modifying the database. I think anything with an UPDATE, EMPTY, DROP, or INSERT should be suspect. As a worse case scenario, I imagine someone reading on some blog that some DROP or EMPTY is a good idea and they add it without understanding what it does. At least a confirmation warning would alert an admin that they were creating a potentially dangerous and irreversible activity such that they should have a good backup of the database. Peace - Anthony

p.s. - I took the leading spaces out of the component name in the tracker so it is now alphabetical and set all issues to be assigned to the component lead (you) rather than the project lead (me). Thanks for covering the bases (tracker, CVS, download, M&P, and forum).

Activity

Hide
Tim Hunt added a comment -

Did you look at the code, or try installing it and creating a malicious query? I thought I had made it impossible to define a query that altered the database.

However I was not aware of EMPTY, and I forgot TRUNCATE. I should probably add those to the bad words list.

Even so, it is probably safe, because get_records SQL appends LIMIT 5000 to the query, which breaks almost any update query.

Show
Tim Hunt added a comment - Did you look at the code, or try installing it and creating a malicious query? I thought I had made it impossible to define a query that altered the database. However I was not aware of EMPTY, and I forgot TRUNCATE. I should probably add those to the bad words list. Even so, it is probably safe, because get_records SQL appends LIMIT 5000 to the query, which breaks almost any update query.
Hide
Anthony Borrow added a comment -

Tim - I had downloaded and just looked at functionality. I had not noticed the bad word check with report_customsql_bad_words_list() in locallib.php so the bases are covered. If you want you can use this issue to add the EMPTY and TRUNCATE to the bad word list. UPDATE is already there. Feel free to resolve/close as you see fit. Peace - Anthony

Show
Anthony Borrow added a comment - Tim - I had downloaded and just looked at functionality. I had not noticed the bad word check with report_customsql_bad_words_list() in locallib.php so the bases are covered. If you want you can use this issue to add the EMPTY and TRUNCATE to the bad word list. UPDATE is already there. Feel free to resolve/close as you see fit. Peace - Anthony
Hide
Tim Hunt added a comment -

OK, added TRUNCATE to the bad words list.

I think you have made up EMPTY. It does not appear in the list of reserved words: http://docs.moodle.org/en/Development:XMLDB_reserved_words

Oh, and thanks for fixing the component, etc.

Show
Tim Hunt added a comment - OK, added TRUNCATE to the bad words list. I think you have made up EMPTY. It does not appear in the list of reserved words: http://docs.moodle.org/en/Development:XMLDB_reserved_words Oh, and thanks for fixing the component, etc.
Hide
Anthony Borrow added a comment -

Tim - Yes, there is only TRUNCATE sorry I was thinking in phpMyAdmin terminology rather than SQL. Peace - Anthony

Show
Anthony Borrow added a comment - Tim - Yes, there is only TRUNCATE sorry I was thinking in phpMyAdmin terminology rather than SQL. Peace - Anthony
Hide
Anthony Borrow added a comment -

p.s. - I noticed a typo in the commit when you added TRUNCATE which I fixed and committed by adding the comma

Show
Anthony Borrow added a comment - p.s. - I noticed a typo in the commit when you added TRUNCATE which I fixed and committed by adding the comma
Hide
Tim Hunt added a comment -

Doh! thanks for catching that for me.

Show
Tim Hunt added a comment - Doh! thanks for catching that for me.

People

Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: