Improper escaping of identifiers #327
Description
This package does not properly quote identifiers (table and column names) which contain the identifier delimiter character in them. Here are examples of valid identifier (table or column) names in each DBMS which are improperly quoted by Joomla Database:
*
- MySQL.
foo`baris escaped as`foo`bar`instead of`foo``bar`. - PostgreSQL, SQLite
foo"baris escaped as"foo"bar"instead of"foo""bar". - MS/Azure SQL Server
foo[bar]is escaped as[foo[bar]]instead of[foo[bar]]].
As to whether these are allowed identifier names, yes, they are. For example, according to MySQL's documentation backticks are acceptable characters in table and column names. Not that any sane person would use them, but they are allowed.
The current implementation raises two concerns.
First, the misquoted identifiers cause a SQL error.
Second, and extending from the first, improper escaping allows an attacker to execute arbitrary SQL. For example, assuming a MySQL database and software which chooses table or column names from improperly filtered (e.g. raw instead of cmd Joomla input filtering) user-supplied values an attacker can manipulate the resulting SQL query to inject and execute arbitrary SQL commands.
The fix is straightforward. The DatabaseDriver::quoteNameStr method needs to be changed like so:
if (\strlen($q) === 1) {
if (\strpos($part, $q) !== false)
{
$part = \str_replace($q, $q . $q, $part);
}
$parts[] = $q . $part . $q;
} else {
if (\strpos($part, $q[1]) !== false)
{
$part = \str_replace($q[1], $q[1] . $q[1], $part);
}
$parts[] = $q[0] . $part . $q[1];
}Here's how this works in the supported database servers:
MySQL
Escape a backtick by writing it as two consecutive backticks. Reference.
Identifier quote characters can be included within an identifier if you quote the identifier. If the character to be included within the identifier is the same as that used to quote the identifier itself, then you need to double the character. The following statement creates a table named a`b that contains a column named c"d:
CREATE TABLE `a``b` (`c"d` INT);
(Side note: I find it really funny that GitHub's MySQL highlighter is thrown off by the actually valid form of the MySQL code.)
PostgreSQL
Escape a double quote by writing it as two consecutive double quotes. Reference.
Quoted identifiers can contain any character, except the character with code zero. (To include a double quote, write two double quotes.)
Microsoft SQL Server / Azure SQL Server
Only the closing square bracket needs to be escaped as two consecutive square brackets. Reference.
SELECT QUOTENAME('abc[]def');
[abc[]]def]
(1 row(s) affected)
SQLite
Escape a double quote by writing it as two consecutive double quotes. Reference.
The documentation is a bit cryptic. We are told that SQLite follows the SQL standard, and that the identifier delimiter is a double-quote. The SQL standard defines the escape of the identifier delimiter inside an identifier string as being the identifier delimiter typed twice in a row.