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

auth_ldap_bulk_insert() fails when user data contains single quotes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 1.6.3, 1.7
    • Fix Version/s: 1.6.4, 1.7.1, 1.8
    • Component/s: Authentication
    • Labels:
      None
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_16_STABLE, MOODLE_17_STABLE
    • Fixed Branches:
      MOODLE_16_STABLE, 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

              Assignee:
              skodak Petr Skoda
              Reporter:
              iarenaza Iñaki Arenaza
              Tester:
              Nobody
              Participants:
              Component watchers:
              Adrian Greeve, Jake Dallimore, Mathew May, Mihail Geshoski, Peter Dias
              Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Fix Release Date:
                17/Jan/07