Mitigate “Magellan vulnerabilitites” in PHP 7.2?

  104438
February 15, 2019 14:00 cmbecker69@gmx.de ("Christoph M. Becker")
Hi!

You may have heard about the so called “Magellan vulnerabilities”[1]
which potentially affect scripts which allow untrusted users to execute
almost arbitrary SQL queries.  BohwaZ provided a pull request[2] which
introduces an ini setting which enables defenses built-in to SQLite ≥
3.26.0 against the corruption of tables via SQL.

In my opinion, adding this ini setting to PHP-7.4 is a no brainer, but I
suggest that we backport it to PHP-7.2 as well.

And likely we should offer something of this kind for PDO as well.  Not
sure if a driver specific ini setting would be suitable.  Suggestions
welcome!

Thoughts?

[1] <https://blade.tencent.com/magellan/index_en.html>
[2] <https://github.com/php/php-src/pull/3709>

-- 
Christoph M. Becker
  104439
February 15, 2019 14:54 php@bohwaz.net (BohwaZ/PHP)
Thanks Christoph!

Just to be clear, this patch doesn't prevent security issues if you 
don't update your SQLite3 library, it just implements a new option 
available in newer SQLite versions which will prevent arbitrary changes 
to the internals of a SQLite database only if you SQLite3 library is 
3.26+. Changing the internals of a SQLite database is something that 
should be quite rare in the real world I think.

This addresses potential security issues for PHP applications allowing 
end-users to run arbitrary SQL queries.

But please note that if your application does allow end-users to run 
arbitrary SQL queries, I advise that you limit them to read-only by 
either:

* opening the database as readonly (available in PDO since PHP 7.3: use 
open attribute: PDO::SQLITE_ATTR_OPEN_FLAGS => PDO::SQLITE_READONLY)
* using the SQLite3Stmt::readOnly method (will tell you if a prepared 
statement will write to the database)
* or using the equivalent for PDO:

$st = $pdo->prepare('SELECT * FROM table;');
var_dump($st->getAttribute(PDO::SQLITE_ATTR_READONLY_STATEMENT));

This last one should be available in PHP 7.4 I hope? See 
https://github.com/php/php-src/pull/2760

Both PDO features are currently undocumented but it's on my TODO list as 
well.

If your users are performing custom SELECT queries, this is the best 
thing to do.

I am now working on bringing support for a userland custom callback to 
the SQLite3 authorizer API, this will allow PHP code to restrict access 
to specific tables, columns and operations, that should also improve 
security in the future.

As a side note, although my time is quite limited, I have worked for the 
last two years to improve features support and security of the SQLite3 
extension and try to match features of the SQLite3 extension in PDO as 
well.

I have to give a warm thank you to Christoph and everyone else who 
helped me in this :)
And if anyone is interested in helping me improve SQLite support in PHP 
you are more than welcome :)

Cheers.
  104466
February 19, 2019 01:16 smalyshev@gmail.com (Stanislav Malyshev)
Hi!

> In my opinion, adding this ini setting to PHP-7.4 is a no brainer, but I > suggest that we backport it to PHP-7.2 as well.
I don't see a reason why not - if the option is useful for improving security/stability, let's backport it. If it's security related, maybe even to 7.1 since it's still in security support (if it's not too hard). -- Stas Malyshev smalyshev@gmail.com
  104659
March 11, 2019 16:35 cmbecker69@gmx.de ("Christoph M. Becker")
On 19.02.2019 at 02:16, Stanislav Malyshev wrote:

>> In my opinion, adding this ini setting to PHP-7.4 is a no brainer, but I >> suggest that we backport it to PHP-7.2 as well. > > I don't see a reason why not - if the option is useful for improving > security/stability, let's backport it. If it's security related, maybe > even to 7.1 since it's still in security support (if it's not too hard).
I have applied the PR[1] to PHP-7.2+. Joe may consider to cherry-pick to 7.1. [1] <https://github.com/php/php-src/pull/3709> -- Christoph M. Becker
  104660
March 11, 2019 17:02 krakjoe@gmail.com (Joe Watkins)
Cheery picked into 7.1

Cheers
Joe

On Mon, 11 Mar 2019 at 17:35, Christoph M. Becker <cmbecker69@gmx.de> wrote:

> On 19.02.2019 at 02:16, Stanislav Malyshev wrote: > > >> In my opinion, adding this ini setting to PHP-7.4 is a no brainer, but I > >> suggest that we backport it to PHP-7.2 as well. > > > > I don't see a reason why not - if the option is useful for improving > > security/stability, let's backport it. If it's security related, maybe > > even to 7.1 since it's still in security support (if it's not too hard). > > I have applied the PR[1] to PHP-7.2+. Joe may consider to cherry-pick > to 7.1. > > [1] <https://github.com/php/php-src/pull/3709> > > -- > Christoph M. Becker >