Prevent Identifier breakout via quoteSql() sequences

CHANGELOG.md

@@ -10,7 +10,7 @@
   `getSchemaDefaultValues()`, `getSchemaForeignKeys()`, `getSchemaIndexes()`, `getSchemaPrimaryKeys()`,
   `getSchemaUniques()` and `getTableSchemas()` expose the table each item belongs to (@KalimeroMK)
 - New #1152: Add `UuidValue` expression that represents a UUID value independently of DBMS (@KalimeroMK)
-- New #1204: Add `Identifier` expression for strict identifier quoting (@vjik)
+- New #1204, #1207: Add `Identifier` expression for strict identifier quoting (@vjik)
 - New #1204: Add `IdentifierQuoterInterface` and implement it in `Quoter` (@vjik)
 - Enh #1152: Improve the exception message of `DbUuidHelper::toUuid()` when the value isn't a valid UUID
   (@KalimeroMK)

docs/guide/en/expressions/values.md

@@ -55,8 +55,8 @@ new DateTimeValue(new DateTime('2023-01-01 12:00:00'));
 The [Identifier](../../../../src/Expression/Value/Identifier.php) expression allows to represent an identifier, such as
 a column name optionally prefixed with table and schema names, that is always quoted strictly. Unlike
 [ColumnName](#columnname), it never renders an SQL fragment: each name part is enclosed in quote characters of the DBMS,
-quote characters inside the part are escaped, and special sequences (`(`, `*`, `[[...]]`, `{{...}}`, `%`) are quoted
-literally. So it's safe to use with names taken from untrusted input, for example, sorting or filtering field names.
+quote characters inside the part are escaped, and special characters (`(`, `*`, `%`) are quoted literally. So it's safe
+to use with names taken from untrusted input, for example, sorting or filtering field names.
 
 A string name is split into parts by dots. To use a name part that contains a dot, pass the parts as an array.
 
@@ -75,6 +75,15 @@ brackets (`[a]]b]`).
 Empty name parts are allowed, since some DBMS (for example, SQLite) support empty identifiers: `new Identifier('')`
 renders `""`. An empty array causes `InvalidArgumentException`.
 
+The following can't be used in name parts and cause `InvalidArgumentException` when building SQL:
+
+- `[[...]]`, for example, `[[name]]`;
+- `{{...}}`, for example, `{{table}}` or `{{%table}}`;
+- for MSSQL, `[[...]`, for example, `[[name]`;
+- for MSSQL, `[...]` at the beginning of a part, for example, `[name]`.
+
+Here `...` is a sequence of letters, digits, underscores, hyphens, dots and spaces.
+
 ## JsonValue
 
 The [JsonValue](../../../../src/Expression/Value/JsonValue.php) expression allows to represent a PHP value as 

src/Expression/Value/Builder/IdentifierBuilder.php

@@ -4,12 +4,14 @@
 
 namespace Yiisoft\Db\Expression\Value\Builder;
 
+use InvalidArgumentException;
 use Yiisoft\Db\Exception\NotSupportedException;
 use Yiisoft\Db\Expression\ExpressionBuilderInterface;
 use Yiisoft\Db\Expression\ExpressionInterface;
 use Yiisoft\Db\Expression\Value\Identifier;
 use Yiisoft\Db\QueryBuilder\QueryBuilderInterface;
 use Yiisoft\Db\Schema\IdentifierQuoterInterface;
+use Yiisoft\Db\Schema\QuoterInterface;
 
 use function sprintf;
 
@@ -19,6 +21,15 @@
  * This builder converts {@see Identifier} expressions into strictly quoted identifiers using
  * {@see IdentifierQuoterInterface::quoteIdentifier()} of the query builder quoter.
  *
+ * The following can't be used in name parts and cause `InvalidArgumentException`:
+ *
+ * - `[[...]]`, for example, `[[name]]`;
+ * - `{{...}}`, for example, `{{table}}` or `{{%table}}`;
+ * - for MSSQL, `[[...]`, for example, `[[name]`;
+ * - for MSSQL, `[...]` at the beginning of a part, for example, `[name]`.
+ *
+ * Here `...` is a sequence of letters, digits, underscores, hyphens, dots and spaces.
+ *
  * @implements ExpressionBuilderInterface<Identifier>
  */
 final class IdentifierBuilder implements ExpressionBuilderInterface
@@ -31,6 +42,8 @@ public function __construct(
     ) {}
 
     /**
+     * @throws InvalidArgumentException If the quoted identifier contains sequences processed by
+     * {@see QuoterInterface::quoteSql()}.
      * @throws NotSupportedException If the quoter doesn't implement {@see IdentifierQuoterInterface}.
      */
     public function build(ExpressionInterface $expression, array &$params = []): string
@@ -48,6 +61,18 @@ public function build(ExpressionInterface $expression, array &$params = []): str
             );
         }
 
-        return $quoter->quoteIdentifier(...$expression->parts);
+        $sql = $quoter->quoteIdentifier(...$expression->parts);
+
+        if ($quoter->quoteSql($sql) !== $sql) {
+            throw new InvalidArgumentException(
+                sprintf(
+                    'Identifier %s must not contain sequences processed by "%s::quoteSql()", such as "[[...]]" or "{{...}}".',
+                    $sql,
+                    QuoterInterface::class,
+                ),
+            );
+        }
+
+        return $sql;
     }
 }

src/Expression/Value/Identifier.php

@@ -17,8 +17,7 @@
  * Unlike {@see ColumnName}, which uses heuristic quoting and keeps names containing `(`, `[[`, `{{` or starting
  * with a quote character as is, this expression always renders a properly quoted identifier and never an SQL fragment.
  * Each name part is enclosed in quote characters of the DBMS, quote characters inside the part are escaped, and special
- * sequences (`(`, `*`, `[[...]]`, `{{...}}`, `%`) are quoted literally. So it's safe to use with names taken from
- * untrusted input:
+ * characters (`(`, `*`, `%`) are quoted literally. So it's safe to use with names taken from untrusted input:
  *
  * ```php
  * new Identifier('name'); // "name"
@@ -33,6 +32,15 @@
  * ```php
  * new Identifier(['t', 'na.me']); // "t"."na.me"
  * ```
+ *
+ * The following can't be used in name parts and cause `InvalidArgumentException` when building SQL:
+ *
+ * - `[[...]]`, for example, `[[name]]`;
+ * - `{{...}}`, for example, `{{table}}` or `{{%table}}`;
+ * - for MSSQL, `[[...]`, for example, `[[name]`;
+ * - for MSSQL, `[...]` at the beginning of a part, for example, `[name]`.
+ *
+ * Here `...` is a sequence of letters, digits, underscores, hyphens, dots and spaces.
  */
 final class Identifier implements ExpressionInterface
 {

tests/Db/Expression/Value/Builder/IdentifierBuilderTest.php

@@ -4,11 +4,14 @@
 
 namespace Yiisoft\Db\Tests\Db\Expression\Value\Builder;
 
+use InvalidArgumentException;
+use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\TestCase;
 use Yiisoft\Db\Exception\NotSupportedException;
 use Yiisoft\Db\Expression\Value\Builder\IdentifierBuilder;
 use Yiisoft\Db\Expression\Value\Identifier;
 use Yiisoft\Db\QueryBuilder\QueryBuilderInterface;
+use Yiisoft\Db\Schema\Quoter;
 use Yiisoft\Db\Schema\QuoterInterface;
 use Yiisoft\Db\Tests\Support\TestHelper;
 
@@ -54,4 +57,50 @@ public function testBuildWithNotSupportedQuoter(): void
 
         $builder->build(new Identifier('name'));
     }
+
+    public static function dataBuildWithQuoteSqlSequences(): iterable
+    {
+        yield 'double quotes, square brackets' => ['"', '[[name]]', '"[[name]]"'];
+        yield 'double quotes, curly brackets' => ['"', '{{t}}', '"{{t}}"'];
+        yield 'double quotes, curly brackets with prefix' => ['"', '{{%t}}', '"{{%t}}"'];
+        yield 'double quotes, square brackets inside' => ['"', 'x[[a b]]y', '"x[[a b]]y"'];
+        yield 'square brackets, square brackets' => [['[', ']'], '[name]', '[[name]]]'];
+        yield 'square brackets, square brackets inside' => [['[', ']'], '[w] - 1 - [z', '[[w]] - 1 - [z]'];
+    }
+
+    #[DataProvider('dataBuildWithQuoteSqlSequences')]
+    public function testBuildWithQuoteSqlSequences(string|array $quoteCharacter, string $name, string $expected): void
+    {
+        $queryBuilder = $this->createStub(QueryBuilderInterface::class);
+        $queryBuilder->method('getQuoter')->willReturn(new Quoter($quoteCharacter, $quoteCharacter));
+        $builder = new IdentifierBuilder($queryBuilder);
+        $identifier = new Identifier([$name]);
+
+        $this->expectException(InvalidArgumentException::class);
+        $this->expectExceptionMessage(
+            'Identifier ' . $expected . ' must not contain sequences processed by '
+            . '"Yiisoft\Db\Schema\QuoterInterface::quoteSql()", such as "[[...]]" or "{{...}}".',
+        );
+        $builder->build($identifier);
+    }
+
+    public static function dataBuildWithBrackets(): iterable
+    {
+        yield 'double quotes, square brackets' => ['"', '[name]', '"[name]"'];
+        yield 'double quotes, curly brackets' => ['"', '{name}', '"{name}"'];
+        yield 'double quotes, empty square brackets' => ['"', '[[]]', '"[[]]"'];
+        yield 'double quotes, square brackets with comma' => ['"', '[[a,b]]', '"[[a,b]]"'];
+        yield 'square brackets, closing bracket' => [['[', ']'], 'na]me', '[na]]me]'];
+        yield 'square brackets, opening bracket' => [['[', ']'], '[name', '[[name]'];
+    }
+
+    #[DataProvider('dataBuildWithBrackets')]
+    public function testBuildWithBrackets(string|array $quoteCharacter, string $name, string $expected): void
+    {
+        $queryBuilder = $this->createStub(QueryBuilderInterface::class);
+        $queryBuilder->method('getQuoter')->willReturn(new Quoter($quoteCharacter, $quoteCharacter));
+        $builder = new IdentifierBuilder($queryBuilder);
+
+        $this->assertSame($expected, $builder->build(new Identifier([$name])));
+    }
 }