Re: Sql Object Model Parser & Sanitizer (was [RFC] is_literal)
| From: | Mike Schinkel | Date: | Tue, 22 Jun 2021 16:14:48 +0000 |
| Subject: | Re: Sql Object Model Parser & Sanitizer (was [RFC] is_literal) | ||
| References: | 1 2 3 4 5 6 | Groups: | php.internals |
| Request: | Send a blank email to internals+get-115032@lists.php.net to get a copy of this message | ||
> On Jun 22, 2021, at 11:56 AM, Pierre <pierre-php@processus.org> wrote:
>
> Le 22/06/2021 à 17:35, Mike Schinkel a écrit :
>> ¸ÜìSƒ¿B}
>> ôÝ‚²Ñéhttps://github.com/WordPress/WordPress/blob/master/wp-includes/wp-db.php#L2050
> Sorry for the discussion pollution here but, but ouch, plugins are still using this unsafe form
> ? Reminds when I was a student, I learnt to parametrize queries there, it was 20 years ago. I never
> understood people not doing that in the first place.
>> But also because of much legacy code exists in the form of plugins and themes that do not
>> support parameterized queries.
> Yes I agree, legacy is legacy, you have to deal with it. But all legacy code cannot be fixed,
> and doing a highly complex SQL parsing / escaping / vulnerability detection code that explicitly
> targets legacy code and not modern code seems weird to me.
>> HOWEVER, whether mysqli supports parameterised queries or not is all a moot point because
>> parameterised queries do not allow for parameterizing field names or table names. And the point of
>> this thread was to discuss how to mark SQL that has been composed at run-time to be
>> "safe." Without being able to parameterize field names and table names parameterised
>> queries are not a sufficiently complete solution.
>
> Not being able to parametrize table or field names is not only a problem for mysqli, but it is
> for PDO and pgsql too. That's a place where userland query-builders and others DBALs, even the
> most basic ones do shine, and brings a real added-value.
>
> But having anyone, writing SQL with user-given table names or column names, and executing it
> using something like WP's _do_query() method seems like they *WANT* to be hacked.
Are you not familiar with PHPMyAdmin[1] and/or Adminer[2]?
Do they, or anyone else who has a similar use-case want to be hacked?
> I'm not sure how you will succeed in plugging the is_trusted() / is_literal() /
> is_wathever() method correctly in an SQL Model Parser & Sanitizer anyway, knowing that at this
> point, all you'll receive is a huge string issued by some plugin API which has already done
> crazy dark (and probably bugguy as well) magic.
>
> I don't see how adding magic in PHP core will avoid the need to fix all those legacy
> plugins, they probably would need themselves to use this new shiny API to benefit from it ? In the
> opposite, if something alters the behavior of mysqli implicitly for everyone in order to make it
> safe, it sounds like there will be a lot of BC ? In both case, it seems that it will not do any
> shiny magic to me.
It would not affect *any* of those plugins by itself. They would be left to fend for their own.
What it *would* do is allow those developers writing new sites and/or plugins or maintaining old
ones who proactively choose to sanitize their SQL to be able to do so before they pass their SQL to
$wpdb->query(), especially if the RFC is_trusted() and/or is_literal() passes.
> But, I might be wrong, this thread becomes harder and harder to read, and I may have missed a
> few points.
It just started! Note I created a new thread from the is_literal()/is_trusted() thread to talk
about SQL.
-Mike
[1] https://www.phpmyadmin.net/
[2] https://www.adminer.org/