Tweaking Moodle book search

A couple of weeks ago I gave a presentation showing off some work from the Moodle open Book project. The middle of the presentation was a live demonstration of the Moodle Book and various features. At one point in the presentation members of the audience (including a number of academics who used the Book module in their Moodle sites) gave an audible gasp. This occurred when I showed off the search block for the Book module. A tool that allows the user to search the contents of all the books in a Moodle course. The gasp indicated just how much teachers and students desire this feature. A feature I’ve been calling out for quite some time.

The GitHub repository has been around for 2 years. So why isn’t this block more widely available in Moodle sites? There’s a security flaw in the code and is somewhat unfinished.

The Moodle SQL injection page outlines at least two broad steps that Moodle code should take to prevent a nefarious person from gaining inappropriate access. These are

  1. Using appropriate parameter cleaning mechanisms on data coming from outside of Moodle (e.g. search terms entered into a form).
  2. Using provided Moodle functions to retrieve data from the database (e.g. search the database for content in a Moodle book)

The Moodle book search block currently meets #1, but fails #2.

The following aims to explore and hopefully remedy this problem.

Current status is that some initial changes have been made to a local version of the block that borrows lessons from the forum search.  Need to spend a bit more time on this, but it’s on the way.

The form

The form for the block currently passes the following information.

Added by the block code

  • courseid; and
  • page.

The user is able to enter data into: bsquery

The processing

Apart from standard processing the main searching is done in a function named search which

  • Deals with some apparent differences between flavours of SQL between databases.
    Seeming to point to a problem in how it’s engaging with databases. DOES FORUM SEARCH DO THIS
  • Focuses attention on books the user is allowed to read.
  • Generates strings containing SQL statement
    See below for the format.

    • Supports + and –
  • Uses get_records_sql to retrieve any matches
    Not using placeholders, which is a problem.

A search for “copyright” generates a SQL statement similar to the following

SELECT DISTINCT bc.* FROM mdl_book_chapters bc, mdl_book b 
        WHERE b.course = 12 AND b.id IN (..long list of book ids) 
        AND bc.bookid = b.id AND bc.hidden = 0 AND 
        (( bc.title ILIKE '%copyright%' ) OR 
         ( bc.content ILIKE '%copyright%' ) ) 
        ORDER BY bc.bookid, bc.pagenum

Each word added to the search phrase adds options to the “ilikes”

( bc.title ILIKE ‘%copyright%’ AND bc.title ILIKE ‘%creative%’
AND bc.title ILIKE ‘%commons%’ )

Questions include

  • Can get_records_sql be replaced with get_records
  • Can the database dependency be removed
  • Can placeholders be used

Comparison with forum search

The forum search is an accepted part of Moodle.  Checking how it works might provide some inspiration to copy.

Has a slight be compartmentalised structure

   $forums = forum_get_readable_forums($USER->id, $courseid);

Has to deal with a lot more complexity than the book search.

Uses $DB->get_in_or_equal

Makes use of a method search_generate_SQL to do as the name suggests.  This is something that should be worked into the Book search block.

Initial testing and that is working.  Dig a bit and it makes sense.  Also seems to tidy up the code a fair bit.

 

One thought on “Tweaking Moodle book search

  1. Pingback: Finishing tweaks to Moodle book search block | The Weblog of (a) David Jones

Leave a Reply

Please log in using one of these methods to post your comment:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s