tursodatabase / turso-driver-laravel

Turso Driver for Laravel with Native libSQL
https://turso.tech/sdk/php/guides/laravel
MIT License
56 stars 6 forks source link

[Bug]: String quoting #10

Open ineersa opened 2 months ago

ineersa commented 2 months ago

What happened?

Unsafe strings quoting.

I think that such features should be implemented inside driver, similar to https://github.com/php/php-src/blob/master/ext/pdo_sqlite/sqlite_driver.c#L223

This opens security vulnerabilities for this library separate from sqlite3 and libsql. Also this code is duplicated in 2 classes, which can lead to errors.

How to reproduce the bug

public static function escapeString($value)
    {
        // DISCUSSION: Open PR if you have best approach
        $escaped_value = str_replace(
            ['\\', "\x00", "\n", "\r", "\x1a", "'", '"'],
            ['\\\\', '\\0', '\\n', '\\r', '\\Z', "\\'", '\\"'],
            $value
        );

        return $escaped_value;
    }

    public function quote(string $value): string
    {
        return self::escapeString($value);
    }

Package Version

latest

PHP Version

8.3.8

Laravel Version

11

Which operating systems does with happen with?

Linux

Notes

No response

ineersa commented 1 month ago

@darkterminal I've added some basic unit test for your new approach with strtr - https://github.com/tursodatabase/turso-driver-laravel/pull/15/files#diff-66fb0fca9b804fc81e208d29c1eaf187c0e2c7a06aa5934f13871cefdd1b8440R34

This approach is very similar to https://www.php.net/manual/en/mysqli.real-escape-string.php and actually can have problems with some character sets same as built in PHP function.

Also I'm kinda confused why you are rejecting SQLite way of doing it, via using internal SQLite quoting mechanism in extension.

Also from SQLite documentation - https://www.sqlite.org/lang_expr.html

A string constant is formed by enclosing the string in single quotes ('). A single quote within the string can be encoded by putting two single quotes in a row - as in Pascal. C-style escapes using the backslash character are not supported because they are not standard SQL.

So single quotes should be doubled instead of backslash escaping.

darkterminal commented 1 month ago

It's mentioned here