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

CLONE -auth_ldap_bulk_insert() potential problem for MSSQL and Oracle

    XMLWordPrintable

Details

    • Bug
    • Status: Closed
    • Minor
    • Resolution: Fixed
    • 1.7
    • 1.8
    • Authentication
    • None
    • Microsoft SQL, Oracle
    • MOODLE_17_STABLE
    • MOODLE_18_STABLE

    Description

      In auth_sync_users() we user auth_ldap_bulk_insert() to insert temporary user data into the database. Depending on the underlying database, we insert one value at a time, or a number of them (1000 by default).

      Inside the function, in order to quote all the values before sending them to the database, we execute the following code:

      // make those values safe
      array_map('addslashes', $users);

      but this is wrong. array_map() doesn't modify the array argument, but instead it returns a modified array (see http://www.php.net/array_map). So the code should be:

      // make those values safe
      $users = array_map('addslashes', $users);

      If there is a username with a single quote in it, the value is not scaped and the insert fails silently (we don't check the return code in the next execute_sql() call). This in turn makes a lot of external users 'inexistent' and their corresponding internal users are deleted from Moodle.

      See http://moodle.org/mod/forum/discuss.php?d=59753 for more details on this.

      By the way, currently auth_sync_users() just tests for MySQL and Postgresql explicitly, and set bulk_insert_records = 1 just for Postgresl, so Oracle and MS SQL get the defauknowledge
      lt of 1000. I don't know if Oracle and MS SQL allow the extended insert syntax or not, so anyone who knows Oracle and MS SQL better should have a look at it.

      Saludos. Iñaki.

      Attachments

        Issue Links

          Activity

            People

              stronk7 Eloy Lafuente (stronk7)
              iarenaza Iñaki Arenaza
              Nobody Nobody
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Sujith Haridasan
              Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:
                31/Mar/07