sqlite PDO::quote silently corrupts strings with null bytes

[divinity76]

Description

The following code:

<?php

declare(strict_types=1);

$db = new \PDO('sqlite::memory:', null, null, array(
    \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
    \PDO::ATTR_DEFAULT_FETCH_MODE => \PDO::FETCH_ASSOC,
    \PDO::ATTR_EMULATE_PREPARES => false,
));
var_dump($db->quote("foo\x00bar"));

Resulted in this output:

string(5) "'foo'"

But I expected this output instead:

string(17) "x'666f6f00626172'"

-or-

Fatal error: Uncaught ValueError: PDO::quote(): Argument #1 ($string) must not contain any null bytes

(but preferably the former)

• but SILENTLY CORRUPTING THE STRING is certainly not what I expected, and not the appropriate course of action.

PHP Version

PHP 8.3.4

Operating System

Ubuntu 24.04-beta

[nielsdos]

Lovely, the implementation even has a comment above it stating it is broken:

php-src/ext/pdo_sqlite/sqlite_driver.c

Lines 222 to 235 in 08b2ab2

	/* NB: doesn't handle binary strings... use prepared stmts for that */
	static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
	{
	char *quoted;
	if (ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2) {
	return NULL;
	}
	quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
	/* TODO use %Q format? */
	sqlite3_snprintf(2*ZSTR_LEN(unquoted) + 3, quoted, "'%q'", ZSTR_VAL(unquoted));
	zend_string *quoted_str = zend_string_init(quoted, strlen(quoted), 0);
	efree(quoted);
	return quoted_str;
	}

[divinity76]

Any idea what (INT_MAX - 3) / 2 is supposed to be? It's 73 million bytes higher than SQLite's SQLITE_MAX_SQL_LENGTH default value,

((0x7FFFFFFF - 3) / 2) - 1,000,000,000 = 73741822

Maybe PHP max string length? Idk

[nielsdos]

It's protection against integer overflow causing a buffer overflow.

[mvorisek]

in atk4/data we use https://github.com/atk4/data/blob/5.0.0/src/Persistence/Sql/Expression.php#L268

[nielsdos]

I think the best solution is to just implement this thing ourselves, looking at the sqlite code it doesn't seem difficult at all.

[divinity76]

proposed fix: #13953

[mvorisek]

I meant concat using x'00' might be better as otherwise the payload/used bandwith can be 2x as high.

[nielsdos]

I agree with @mvorisek
This is what I was writing as a prototype: https://gist.github.com/nielsdos/1fc34c55b2ae7d24e7b2759ae26ce586
It seems to work but I need to do extra tests and cleanup. I'll take a look at @divinity76 's approach now