Issue #3201266 by mondrake, daffie: Protection against multiple statements...

Issue #3201266 by mondrake, daffie: Protection against multiple statements should be moved to prepareStatement

core/lib/Drupal/Core/Database/Connection.php

@@ -538,7 +538,8 @@ public function getFullQualifiedTableName($table) {
    * identifiers enclosed in square brackets.
    *
    * @param string $query
-   *   The query string as SQL, with curly braces surrounding the table names.
+   *   The query string as SQL, with curly braces surrounding the table names,
+   *   and square brackets surrounding identifiers.
    * @param array $options
    *   An associative array of options to control how the query is run. See
    *   the documentation for self::defaultOptions() for details. The content of
@@ -547,14 +548,15 @@ public function getFullQualifiedTableName($table) {
    * @return \Drupal\Core\Database\StatementInterface
    *   A PDO prepared statement ready for its execute() method.
    *
+   * @throws \InvalidArgumentException
+   *   If multiple statements are included in the string, and delimiters are
+   *   not allowed in the query.
    * @throws \Drupal\Core\Database\DatabaseExceptionWrapper
    */
   public function prepareStatement(string $query, array $options): StatementInterface {
-    $query = $this->prefixTables($query);
-    if (!($options['allow_square_brackets'] ?? FALSE)) {
-      $query = $this->quoteIdentifiers($query);
-    }
     try {
+      $query = $this->preprocessStatement($query, $options);
+
       // @todo in Drupal 10, only return the StatementWrapper.
       // @see https://www.drupal.org/node/3177490
       $statement = $this->statementWrapperClass ?
@@ -574,6 +576,52 @@ public function prepareStatement(string $query, array $options): StatementInterf
     return $statement;
   }
 
+  /**
+   * Returns a string SQL statement ready for preparation.
+   *
+   * This method replaces table names in curly braces and identifiers in square
+   * brackets with platform specific replacements, appropriately escaping them
+   * and wrapping them with platform quote characters.
+   *
+   * @param string $query
+   *   The query string as SQL, with curly braces surrounding the table names,
+   *   and square brackets surrounding identifiers.
+   * @param array $options
+   *   An associative array of options to control how the query is run. See
+   *   the documentation for self::defaultOptions() for details.
+   *
+   * @return string
+   *   A string SQL statement ready for preparation.
+   *
+   * @throws \InvalidArgumentException
+   *   If multiple statements are included in the string, and delimiters are
+   *   not allowed in the query.
+   */
+  protected function preprocessStatement(string $query, array $options): string {
+    // To protect against SQL injection, Drupal only supports executing one
+    // statement at a time.  Thus, the presence of a SQL delimiter (the
+    // semicolon) is not allowed unless the option is set.  Allowing semicolons
+    // should only be needed for special cases like defining a function or
+    // stored procedure in SQL. Trim any trailing delimiter to minimize false
+    // positives unless delimiter is allowed.
+    $trim_chars = " \xA0\t\n\r\0\x0B";
+    if (empty($options['allow_delimiter_in_query'])) {
+      $trim_chars .= ';';
+    }
+    $query = rtrim($query, $trim_chars);
+    if (strpos($query, ';') !== FALSE && empty($options['allow_delimiter_in_query'])) {
+      throw new \InvalidArgumentException('; is not supported in SQL strings. Use only one statement at a time.');
+    }
+
+    // Resolve {tables} and [identifiers] to the platform specific syntax.
+    $query = $this->prefixTables($query);
+    if (!($options['allow_square_brackets'] ?? FALSE)) {
+      $query = $this->quoteIdentifiers($query);
+    }
+
+    return $query;
+  }
+
   /**
    * Prepares a query string and returns the prepared statement.
    *
@@ -813,20 +861,6 @@ public function query($query, array $args = [], $options = []) {
     // object, which we pass to StatementInterface::execute.
     if (is_string($query)) {
       $this->expandArguments($query, $args);
-      // To protect against SQL injection, Drupal only supports executing one
-      // statement at a time.  Thus, the presence of a SQL delimiter (the
-      // semicolon) is not allowed unless the option is set.  Allowing
-      // semicolons should only be needed for special cases like defining a
-      // function or stored procedure in SQL. Trim any trailing delimiter to
-      // minimize false positives unless delimiter is allowed.
-      $trim_chars = " \xA0\t\n\r\0\x0B";
-      if (empty($options['allow_delimiter_in_query'])) {
-        $trim_chars .= ';';
-      }
-      $query = rtrim($query, $trim_chars);
-      if (strpos($query, ';') !== FALSE && empty($options['allow_delimiter_in_query'])) {
-        throw new \InvalidArgumentException('; is not supported in SQL strings. Use only one statement at a time.');
-      }
       $stmt = $this->prepareStatement($query, $options);
     }
     elseif ($query instanceof StatementInterface) {

core/lib/Drupal/Core/Database/Driver/sqlite/Connection.php

@@ -428,11 +428,14 @@ public function mapConditionOperator($operator) {
    * {@inheritdoc}
    */
   public function prepareStatement(string $query, array $options): StatementInterface {
-    $query = $this->prefixTables($query);
-    if (!($options['allow_square_brackets'] ?? FALSE)) {
-      $query = $this->quoteIdentifiers($query);
+    try {
+      $query = $this->preprocessStatement($query, $options);
+      $statement = new Statement($this->connection, $this, $query, $options['pdo'] ?? []);
+    }
+    catch (\Exception $e) {
+      $this->exceptionHandler()->handleStatementException($e, $query, $options);
     }
-    return new Statement($this->connection, $this, $query, $options['pdo'] ?? []);
+    return $statement;
   }
 
   public function nextId($existing_id = 0) {

core/tests/Drupal/KernelTests/Core/Database/ConnectionTest.php

@@ -181,13 +181,21 @@ public function testMultipleStatementsForNewPhp() {
   }
 
   /**
-   * Ensure that you cannot execute multiple statements.
+   * Ensure that you cannot execute multiple statements in a query.
    */
-  public function testMultipleStatements() {
+  public function testMultipleStatementsQuery() {
     $this->expectException(\InvalidArgumentException::class);
     Database::getConnection('default', 'default')->query('SELECT * FROM {test}; SELECT * FROM {test_people}');
   }
 
+  /**
+   * Ensure that you cannot prepare multiple statements.
+   */
+  public function testMultipleStatements() {
+    $this->expectException(\InvalidArgumentException::class);
+    Database::getConnection('default', 'default')->prepareStatement('SELECT * FROM {test}; SELECT * FROM {test_people}', []);
+  }
+
   /**
    * Test that the method ::condition() returns a Condition object.
    */