Re: Sql Object Model Parser & Sanitizer (was [RFC] is_literal)

From: 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/

« previous php.internals (#115032) next »