Re: svn commit: r1091262 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

On Mon, Apr 11, 2011 at 9:41 PM, Greg Stein <gstein_at_gmail.com> wrote:
> Woah. When did svn_sqlite__prepare arrive?

$ svnd blame subversion/libsvn_subr/sqlite.c | grep svn_sqlite__prepare
875453 hwright
SVN_ERR(svn_sqlite__prepare(&db->prepared_stmts[stmt_idx], db,
873188 gstein svn_sqlite__prepare(svn_sqlite__stmt_t **stmt,
svn_sqlite__db_t *db,
875770 hwright SVN_ERR(svn_sqlite__prepare(&stmt, db, "PRAGMA
user_version;", scratch_pool));

It looks like you introduced (or resurrected it) in r873188.

> I'm basically -1 on that.
>
> The whole idea behind static statements was to avoid SQL injection attacks.
> Allowing the *code* to construct statements opens us up.
>
> This is Not Good.

This is still a prepared statement, arguments are still bound using
the standard SQLite binding mechanism, so they will be properly
escaped. Yes, one can put arbitrary text in the
statement-to-be-prepared, but the preparation step helps prevent
injection attacks. (You can't prepare multiple statements, which
vastly decreases the attack space.)

Plus, we're talking about local data here. While sql injection is a
Bad Thing, I suspect somebody wanting to hose a working copy will do
something much simpler: rm .svn/wc.db. :)

On a higher-level, if we *don't* use this API, what are your
suggestions on how to create a IN clause with arbitrary numbers of
values?

-Hyrum
Received on 2011-04-12 04:59:25 CEST