Vulnerability to SQL injection even when SQLite3::escapeString() is used and no user input is asked?

Question

I am referring to this answer of mine to another question, which another user criticized because vulnerable to SQL injection, even if no user input is requested and escape procedure is called. The following code is used to create a .sql dump of an SQLite database, using only PHP code with no call to sqlite3 tool (which was the original request of the author of the question).

<?php

$db = new SQLite3(dirname(__FILE__)."/your/db.sqlite");
$db->busyTimeout(5000);

$sql="";

$tables=$db->query("SELECT name FROM sqlite_master WHERE type ='table' AND name NOT LIKE 'sqlite_%';");

while ($table=$tables->fetchArray(SQLITE3_NUM)) {
    $sql.=$db->querySingle("SELECT sql FROM sqlite_master WHERE name = '{$table[0]}'").";\n\n";
    $rows=$db->query("SELECT * FROM {$table[0]}");
    $sql.="INSERT INTO {$table[0]} (";
    $columns=$db->query("PRAGMA table_info({$table[0]})");
    $fieldnames=array();
    while ($column=$columns->fetchArray(SQLITE3_ASSOC)) {
        $fieldnames[]=$column["name"];
    }
    $sql.=implode(",",$fieldnames).") VALUES";
    while ($row=$rows->fetchArray(SQLITE3_ASSOC)) {
        foreach ($row as $k=>$v) {
            $row[$k]="'".SQLite3::escapeString($v)."'";
        }
        $sql.="\n(".implode(",",$row)."),";
    }
    $sql=rtrim($sql,",").";\n\n";
}
file_put_contents("sqlitedump.sql",$sql);

In the comments to this answer, user @Dharman insisted this code is vulnerable, and after asking to provide a full example of a case of how it could lead to problems, he told me to just open a question regarding the matter.

I personally feel there is no way this code could "explode" because of the contents already present inside the database to be dumped, but I'm no authority. So I ask you instead.

What if one of your table names contains a space or a `-` or an SQL reserved word or something? SQL injection is not necessarily malicious or an attempt to access unauthorized data. It's actually more common that SQL injection is a simple mistake which results in a syntax error. — Bill Karwin, Oct 03 '19 at 19:55
This users was referring to `$table[0]` being in the SQL itself. It is a good idea to use prepared statements whenever possible. — Alex Barker, Oct 03 '19 at 20:57
@AlexBarker duly noted, yes I get that now and an escape on `$table[0]` should solve it, I suppose? @BillKarwin and @AlexBarker I would still really like to have a clear-cut example of the worst case scenario where an even galactically unprobable combination of risk factors with table names and columns names/values could create any kind of consequence — ephestione, Oct 04 '19 at 07:27
Would using `'SQLite3::escapeString($table[0])'` everywhere `$table[0]` is inserted inside a query, be an acceptable workaround? — ephestione, Oct 04 '19 at 07:31
Ok, by jumping over several hyperlinks, I found this answer: https://stackoverflow.com/a/8255054/8586810 I am an enthusiast and working with SQL or really anything computer related isn't my job, so I am prone to believe everything that's written in that answer is true, as I couldn't know better anyway. How wrong have I been until now... — ephestione, Oct 04 '19 at 07:57
Really, any question on SO about "SQL injection" should simply be answered with a link to that — ephestione, Oct 04 '19 at 08:15
Still, after some extensive research, even on PHPDelusions.net, I fell back to the belief that, while that response I linked to is the bible for anyone using databases for ONLINE applications, for anything else, small and local-oriented especially, the security benefit doesn't justify the huge overhead. My home alarm system runs off PHP+SQLite, I reverse engineered classes on github to convert them to simple PHP, and the performance benefit is enormous. I don't need it, but I am happier with a snappier system, and all values I get and put in the database are "innocent" — ephestione, Oct 04 '19 at 09:09

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

It's pretty reasonable to name a table Order in an e-commerce application, but this causes a syntax error if you run a query like:

SELECT * FROM Order

Why? Because Order is a reserved keyword in SQLite. It introduces an ORDER BY clause. Using a table named Order in this way just creates a syntax error.

SQLite allows you to name tables after reserved words by delimiting the table name.

This is in the documentation I linked to about reserved words:

'keyword' A keyword in single quotes is a string literal.

"keyword" A keyword in double-quotes is an identifier.

[keyword] A keyword enclosed in square brackets is an identifier. This is not standard SQL. This quoting mechanism is used by MS Access and SQL Server and is included in SQLite for compatibility.

`keyword` A keyword enclosed in grave accents (ASCII code 96) is an identifier. This is not standard SQL. This quoting mechanism is used by MySQL and is included in SQLite for compatibility.

So you can use a table named Order without error like this:

SELECT * FROM "Order"

You shouldn't use SQLite3::escapeString($table[0]) because that's for string literals. Remember from the list above, single-quoted strings are string literals. The escapeString() function only escapes ' characters, so you can put that string inside single-quotes as the delimiter, and thus you can use strings like 'O\'Reilly'.

But escapeString() doesn't do anything for the double-quotes used for delimiting table names. There isn't a function provided for that. So you either have to be sure you have no table names that contain a " character, or else use other PHP functions to escape them.

$safeTable = str_replace('"', '\\"', $table[0]);
$rows = $db->query("SELECT * FROM \"{$safeTable}\"");

You should also read 8. Double-quoted String Literals Are Accepted on this page: https://www.sqlite.org/quirks.html

Thank you for the first "working example of self sql injection" I have read. The lesson I took from it is "never give tables reserved names, but you'd be pretty silly to use \" inside a table name anyway". Confirms my view that for this use case, recurring to prepared statements would be overengineering. — ephestione, Oct 06 '19 at 12:27
Prepared statements wouldn't help anyway, because you can't use query parameters for table names. Some of the comments above got that wrong. Query parameters only take the place of constant values, not identifiers, SQL keywords, expressions, etc. — Bill Karwin, Oct 06 '19 at 17:22

Vulnerability to SQL injection even when SQLite3::escapeString() is used and no user input is asked?

1 Answers1

Linked