diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php index 2baf138f37d..3c736aef71c 100644 --- a/lib/dml/moodle_database.php +++ b/lib/dml/moodle_database.php @@ -690,6 +690,17 @@ abstract class moodle_database { return "\$".$this->fix_sql_params_i; } + /** + * Detects object parameters and throws exception if found + * @param mixed $value + * @return void + */ + protected function detect_objects($value) { + if (is_object($value)) { + throw new coding_exception('Invalid database query parameter value', 'Objects are are not allowed: '.get_class($value)); + } + } + /** * Normalizes sql query parameters and verifies parameters. * @param string $sql The query or part of it. @@ -703,8 +714,9 @@ abstract class moodle_database { // convert table names $sql = $this->fix_table_names($sql); - // cast booleans to 1/0 int + // cast booleans to 1/0 int and detect forbidden objects foreach ($params as $key => $value) { + $this->detect_objects($value); $params[$key] = is_bool($value) ? (int)$value : $value; } diff --git a/lib/dml/mssql_native_moodle_database.php b/lib/dml/mssql_native_moodle_database.php index d5998a7187a..da600cf5dfd 100644 --- a/lib/dml/mssql_native_moodle_database.php +++ b/lib/dml/mssql_native_moodle_database.php @@ -517,6 +517,8 @@ class mssql_native_moodle_database extends moodle_database { * @return mixed the normalised value */ protected function normalise_value($column, $value) { + $this->detect_objects($value); + if (is_bool($value)) { /// Always, convert boolean to int $value = (int)$value; } // And continue processing because text columns with numeric info need special handling below diff --git a/lib/dml/mysqli_native_moodle_database.php b/lib/dml/mysqli_native_moodle_database.php index 242c5c43b9e..2f3ab2197a0 100644 --- a/lib/dml/mysqli_native_moodle_database.php +++ b/lib/dml/mysqli_native_moodle_database.php @@ -651,6 +651,8 @@ class mysqli_native_moodle_database extends moodle_database { * @return mixed the normalised value */ protected function normalise_value($column, $value) { + $this->detect_objects($value); + if (is_bool($value)) { // Always, convert boolean to int $value = (int)$value; diff --git a/lib/dml/oci_native_moodle_database.php b/lib/dml/oci_native_moodle_database.php index b780bd42946..8e10ff1ccb6 100644 --- a/lib/dml/oci_native_moodle_database.php +++ b/lib/dml/oci_native_moodle_database.php @@ -682,6 +682,8 @@ class oci_native_moodle_database extends moodle_database { * @return mixed the normalised value */ protected function normalise_value($column, $value) { + $this->detect_objects($value); + if (is_bool($value)) { // Always, convert boolean to int $value = (int)$value; diff --git a/lib/dml/pgsql_native_moodle_database.php b/lib/dml/pgsql_native_moodle_database.php index d49803b0e8f..ed37e64b199 100644 --- a/lib/dml/pgsql_native_moodle_database.php +++ b/lib/dml/pgsql_native_moodle_database.php @@ -529,6 +529,8 @@ class pgsql_native_moodle_database extends moodle_database { * @return mixed the normalised value */ protected function normalise_value($column, $value) { + $this->detect_objects($value); + if (is_bool($value)) { // Always, convert boolean to int $value = (int)$value; @@ -778,9 +780,10 @@ class pgsql_native_moodle_database extends moodle_database { $fields = implode(',', array_keys($params)); $values = array(); - $count = count($params); - for ($i=1; $i<=$count; $i++) { - $values[] = "\$".$i; + $i = 1; + foreach ($params as $value) { + $this->detect_objects($value); + $values[] = "\$".$i++; } $values = implode(',', $values); @@ -876,6 +879,7 @@ class pgsql_native_moodle_database extends moodle_database { $blobs = array(); foreach ($dataobject as $field=>$value) { + $this->detect_objects($value); if (!isset($columns[$field])) { continue; } @@ -932,6 +936,7 @@ class pgsql_native_moodle_database extends moodle_database { $sets = array(); foreach ($params as $field=>$value) { + $this->detect_objects($value); $sets[] = "$field = \$".$i++; } diff --git a/lib/dml/simpletest/testdml.php b/lib/dml/simpletest/testdml.php index f7c2152b603..39d76420068 100644 --- a/lib/dml/simpletest/testdml.php +++ b/lib/dml/simpletest/testdml.php @@ -3062,6 +3062,117 @@ class dml_test extends UnitTestCase { $this->assertEqual(1, $DB->count_records($tablename)); } + public function test_object_params() { + $DB = $this->tdb; + $dbman = $DB->get_manager(); + + $table = $this->get_test_table(); + $tablename = $table->getName(); + $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null); + $table->add_field('course', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, '0'); + $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id')); + $dbman->create_table($table); + + $o = new stdClass(); // objects without __toString - never worked + try { + $DB->fix_sql_params("SELECT {{$tablename}} WHERE course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + // objects with __toString() forbidden everywhere since 2.3 + $o = new dml_test_object_one(); + try { + $DB->fix_sql_params("SELECT {{$tablename}} WHERE course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $DB->execute("SELECT {{$tablename}} WHERE course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $DB->get_recordset_sql("SELECT {{$tablename}} WHERE course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $DB->get_records_sql("SELECT {{$tablename}} WHERE course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $record = new stdClass(); + $record->course = $o; + $DB->insert_record_raw($tablename, $record); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $record = new stdClass(); + $record->course = $o; + $DB->insert_record($tablename, $record); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $record = new stdClass(); + $record->course = $o; + $DB->import_record($tablename, $record); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $record = new stdClass(); + $record->id = 1; + $record->course = $o; + $DB->update_record_raw($tablename, $record); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $record = new stdClass(); + $record->id = 1; + $record->course = $o; + $DB->update_record($tablename, $record); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $DB->set_field_select($tablename, 'course', 1, "course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + + try { + $DB->delete_records_select($tablename, "course = ? ", array($o)); + $this->fail('coding_exception expected'); + } catch (Exception $e) { + $this->assertTrue($e instanceof coding_exception); + } + } + function test_sql_null_from_clause() { $DB = $this->tdb; $sql = "SELECT 1 AS id ".$DB->sql_null_from_clause(); @@ -4506,3 +4617,13 @@ class moodle_database_for_testing extends moodle_database { public function commit_transaction() {} public function rollback_transaction() {} } + + +/** + * Dumb test class with toString() returrning 1. + */ +class dml_test_object_one { + public function __toString() { + return 1; + } +} \ No newline at end of file diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php index f8c799f4985..914c00edc8a 100644 --- a/lib/dml/sqlsrv_native_moodle_database.php +++ b/lib/dml/sqlsrv_native_moodle_database.php @@ -579,6 +579,8 @@ class sqlsrv_native_moodle_database extends moodle_database { * @return mixed the normalised value */ protected function normalise_value($column, $value) { + $this->detect_objects($value); + if (is_bool($value)) { /// Always, convert boolean to int $value = (int)$value; } // And continue processing because text columns with numeric info need special handling below diff --git a/lib/upgrade.txt b/lib/upgrade.txt index f8f3af8bb23..63c1c105003 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -2,6 +2,12 @@ This files describes API changes in core lbraries and APIs, information provided here is intended especially for developers. +=== 2.3 === + +Database layer changes: +* objects are not allowed in paramters of DML functions, use explicit casting to strings if necessary + + === 2.2 === removed unused libraries: