Skip to content

Commit 1b3d5e0

Browse files
JanJakesJanJakes
authored
Improve identifier and string escaping in the legacy driver (#321)
Improve escaping consistency in the legacy SQLite translator: - Use `quote_identifier()` for all identifier interpolations (table names, column names, index names, trigger names) in SQL queries, token values, and DDL reconstruction output. - Use parameterized queries and `PDO::quote()` for all string literals processed by the translator. See added tests for more details.
1 parent 0650983 commit 1b3d5e0

2 files changed

Lines changed: 261 additions & 79 deletions

File tree

‎tests/WP_SQLite_Translator_Tests.php‎

Lines changed: 149 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,14 +1170,14 @@ public function testColumnWithOnUpdate() {
11701170
'name' => '___tmp_table_created_at_on_update__',
11711171
'tbl_name' => '_tmp_table',
11721172
'rootpage' => '0',
1173-
'sql' => "CREATE TRIGGER \"___tmp_table_created_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"created_at\" = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
1173+
'sql' => "CREATE TRIGGER `___tmp_table_created_at_on_update__`\n\t\t\tAFTER UPDATE ON `_tmp_table`\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE `_tmp_table` SET `created_at` = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
11741174
),
11751175
(object) array(
11761176
'type' => 'trigger',
11771177
'name' => '___tmp_table_updated_at_on_update__',
11781178
'tbl_name' => '_tmp_table',
11791179
'rootpage' => '0',
1180-
'sql' => "CREATE TRIGGER \"___tmp_table_updated_at_on_update__\"\n\t\t\tAFTER UPDATE ON \"_tmp_table\"\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE \"_tmp_table\" SET \"updated_at\" = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
1180+
'sql' => "CREATE TRIGGER `___tmp_table_updated_at_on_update__`\n\t\t\tAFTER UPDATE ON `_tmp_table`\n\t\t\tFOR EACH ROW\n\t\t\tBEGIN\n\t\t\t UPDATE `_tmp_table` SET `updated_at` = CURRENT_TIMESTAMP WHERE rowid = NEW.rowid;\n\t\t\tEND",
11811181
),
11821182
),
11831183
$results
@@ -3557,4 +3557,151 @@ public function testCreateTableWithDefaultNowFunction() {
35573557
$result = $this->assertQuery( 'SELECT * FROM test_now_default WHERE id = 2' );
35583558
$this->assertRegExp( '/\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d/', $result[0]->updated );
35593559
}
3560+
3561+
public function testQuoteIdentifierEscapesBackticks() {
3562+
// Create a table with a backtick in the column name using double-quote
3563+
// quoting (MySQL syntax). The translator must properly escape the
3564+
// backtick when generating SQLite DDL with backtick-quoted identifiers.
3565+
$this->assertQuery(
3566+
'CREATE TABLE _tmp_backtick_test (
3567+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3568+
"col`name" varchar(50) NOT NULL
3569+
);'
3570+
);
3571+
3572+
$this->assertQuery( "INSERT INTO _tmp_backtick_test (ID, \"col`name\") VALUES (1, 'value1')" );
3573+
3574+
$result = $this->assertQuery( 'SELECT * FROM _tmp_backtick_test WHERE ID = 1' );
3575+
$this->assertCount( 1, $result );
3576+
$this->assertEquals( 'value1', $result[0]->{'col`name'} );
3577+
3578+
// Verify the column appears in DESCRIBE output.
3579+
$description = $this->assertQuery( 'DESCRIBE _tmp_backtick_test' );
3580+
$column_names = array_map(
3581+
function ( $row ) {
3582+
return $row->Field;
3583+
},
3584+
$description
3585+
);
3586+
$this->assertContains( 'col`name', $column_names );
3587+
3588+
// Verify SHOW CREATE TABLE produces valid, parseable output.
3589+
$create = $this->assertQuery( 'SHOW CREATE TABLE _tmp_backtick_test' );
3590+
$create_sql = $create[0]->{'Create Table'};
3591+
$this->assertStringContainsString( '`col``name`', $create_sql );
3592+
3593+
// Verify autoincrement detection works with backtick-quoted identifiers.
3594+
$this->assertStringContainsString( 'AUTO_INCREMENT', $create_sql );
3595+
}
3596+
3597+
public function testDoubleQuotedStringsAreParameterized() {
3598+
$this->assertQuery( 'INSERT INTO _options (option_name, option_value) VALUES ("dq_name", "dq_value")' );
3599+
3600+
// The double-quoted strings should be bound as parameters, not inlined.
3601+
$insert_query = null;
3602+
foreach ( $this->engine->executed_sqlite_queries as $q ) {
3603+
if ( stripos( $q['sql'], 'INSERT' ) !== false && stripos( $q['sql'], '_options' ) !== false ) {
3604+
$insert_query = $q;
3605+
break;
3606+
}
3607+
}
3608+
$this->assertNotNull( $insert_query );
3609+
$this->assertNotEmpty( $insert_query['params'], 'Double-quoted strings should be bound as parameters' );
3610+
$this->assertStringNotContainsString( 'dq_name', $insert_query['sql'], 'Value should not appear in SQL' );
3611+
$this->assertStringNotContainsString( 'dq_value', $insert_query['sql'], 'Value should not appear in SQL' );
3612+
$this->assertContains( 'dq_name', $insert_query['params'] );
3613+
$this->assertContains( 'dq_value', $insert_query['params'] );
3614+
3615+
// Verify the data was inserted correctly.
3616+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "dq_name"' );
3617+
$this->assertCount( 1, $result );
3618+
$this->assertEquals( 'dq_value', $result[0]->option_value );
3619+
}
3620+
3621+
public function testDoubleQuotedStringWithBackslashEscapeDoesNotCauseInjection() {
3622+
// In MySQL, \" inside double-quoted strings is an escaped double quote.
3623+
// The MySQL lexer produces a single token: "admin\" OR 1=1--"
3624+
// with value: admin" OR 1=1--
3625+
//
3626+
// Without parameterization, passing the raw token to SQLite would be:
3627+
// "admin\" OR 1=1--" (SQLite sees "admin\" as identifier + SQL)
3628+
//
3629+
// With parameterization, the value is safely bound as a parameter.
3630+
$this->assertQuery(
3631+
'INSERT INTO _options (option_name, option_value) VALUES ("safe_key", "admin\" OR 1=1--")'
3632+
);
3633+
3634+
// Verify the injection payload is not present in the SQL sent to SQLite.
3635+
$insert_query = null;
3636+
foreach ( $this->engine->executed_sqlite_queries as $q ) {
3637+
if ( stripos( $q['sql'], 'INSERT' ) !== false && stripos( $q['sql'], '_options' ) !== false ) {
3638+
$insert_query = $q;
3639+
break;
3640+
}
3641+
}
3642+
$this->assertNotNull( $insert_query );
3643+
$this->assertStringNotContainsString( 'OR 1=1', $insert_query['sql'], 'Injection payload should not appear in SQL' );
3644+
$this->assertNotEmpty( $insert_query['params'], 'Values should be bound as parameters' );
3645+
3646+
$result = $this->assertQuery( 'SELECT * FROM _options WHERE option_name = "safe_key"' );
3647+
$this->assertCount( 1, $result );
3648+
$this->assertEquals( 'admin" OR 1=1--', $result[0]->option_value );
3649+
}
3650+
3651+
public function testDateFormatWithSingleQuotesInFormat() {
3652+
$this->assertQuery(
3653+
'CREATE TABLE _tmp_dates (
3654+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3655+
created_at DATETIME NOT NULL
3656+
);'
3657+
);
3658+
$this->assertQuery( "INSERT INTO _tmp_dates (created_at) VALUES ('2024-01-15 10:30:00')" );
3659+
3660+
// DATE_FORMAT with a format that produces a value — verify it works.
3661+
$result = $this->assertQuery(
3662+
"SELECT DATE_FORMAT(created_at, '%Y-%m-%d') as formatted FROM _tmp_dates"
3663+
);
3664+
$this->assertCount( 1, $result );
3665+
$this->assertEquals( '2024-01-15', $result[0]->formatted );
3666+
}
3667+
3668+
public function testIntervalExpression() {
3669+
$this->assertQuery(
3670+
'CREATE TABLE _tmp_dates (
3671+
ID INTEGER PRIMARY KEY AUTO_INCREMENT NOT NULL,
3672+
created_at DATETIME NOT NULL
3673+
);'
3674+
);
3675+
$this->assertQuery( 'INSERT INTO _tmp_dates (created_at) VALUES (\'2024-01-15 10:30:00\')' );
3676+
3677+
$result = $this->assertQuery(
3678+
'SELECT DATE_ADD(created_at, INTERVAL 1 DAY) as future_date FROM _tmp_dates'
3679+
);
3680+
$this->assertCount( 1, $result );
3681+
$this->assertEquals( '2024-01-16 10:30:00', $result[0]->future_date );
3682+
3683+
$result = $this->assertQuery(
3684+
'SELECT DATE_SUB(created_at, INTERVAL 1 DAY) as past_date FROM _tmp_dates'
3685+
);
3686+
$this->assertCount( 1, $result );
3687+
$this->assertEquals( '2024-01-14 10:30:00', $result[0]->past_date );
3688+
}
3689+
3690+
public function testLikeBinaryWithSingleQuoteInPattern() {
3691+
$this->assertQuery(
3692+
"CREATE TABLE _tmp_table (
3693+
ID INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
3694+
name varchar(50) NOT NULL default ''
3695+
);"
3696+
);
3697+
3698+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('it''s a test')" );
3699+
$this->assertQuery( "INSERT INTO _tmp_table (name) VALUES ('no quote here')" );
3700+
3701+
$result = $this->assertQuery(
3702+
"SELECT * FROM _tmp_table WHERE name LIKE BINARY 'it''s%'"
3703+
);
3704+
$this->assertCount( 1, $result );
3705+
$this->assertEquals( "it's a test", $result[0]->name );
3706+
}
35603707
}

0 commit comments

Comments
 (0)