On Tue, Nov 1, 2016 at 1:58 PM, Tom Bishop, Wenlin Institute
<tangmu(a)wenlin.com> wrote:
Thank you very much for that information!
My impression so far with Database::select() is that it's hard to understand exactly
what goes where, when converting an ordinary SQL query to a set of select() arguments.
It's encouraging, though, to know that you've found it to work fine. I'll keep
working on it. This page --
https://www.mediawiki.org/wiki/Manual:Database_access
-- includes a dead link to --
https://doc.wikimedia.org/mediawiki-core/master/php/classDatabaseBase.html
Any idea where it should link to?
I think I fixed the links. The DatabaseBase class was renamed to
Database a while ago but the docs weren't all updated.
This comment in the talk page is a little scary:
"If you feed an array to select, it's safe. If you construct a string, even a
string to be passed to ::select, then it's up to you to take care of safety."
Based on that, and on the "mostly" in your "has mostly proven
effective", it still seems like prepared statements would be more secure.
Here's a recent article that strongly recommends prepared statements as more secure
than input verification:
http://arstechnica.com/information-technology/2016/10/how-security-flaws-wo…
It says, "Prepared statements are a robust and virtually universal solution to the
problem of SQL injection. Every API worth using supports them, and yet SQL injection flaws
remain in abundance."
Using prepared statements is certainly harder to mess up. Any
framework or application that is starting from scratch today should be
using prepared statements to execute SQL queries. MediaWiki's database
abstraction layer has been around so long that using true prepared
statements wasn't possible with MySQL at the time it was written.
Instead the MediaWiki abstraction tries to use proper quoting and
escaping controlled from the application space. This is delegated to
the underlying database functions whenever possible, but does rely on
the developer being vigilant about properly using
Database::addQuotes() and other escaping functions when making certain
calls.
It would be interesting to see an RfC on converting to a prepared
statement based execution model written by someone who has gone
through our code code and looked at common usage in core and major
extensions. Off the top of my head I have no idea how difficult it
would be to maintain api compatibility and/or to change the callers
that would need to think about constructing a query differently.
Bryan
--
Bryan Davis Wikimedia Foundation <bd808(a)wikimedia.org>
[[m:User:BDavis_(WMF)]] Sr Software Engineer Boise, ID USA
irc: bd808 v:415.839.6885 x6855