apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bojan Smojver <bo...@rexursive.com>
Subject Re: current dbd initiatives
Date Fri, 09 Jun 2006 00:19:00 GMT
Quoting Chris Darroch <chrisd@pearsoncmg.com>:

>    But, suppose someone thinks they can do this safely:
>
> sql = apr_pstrdup(p, "SELECT foo FROM bar WHERE baz = '",
>                   apr_dbd_escape(user_input), "'");
> apr_dbd_prepare(driver, p, handle, sql, NULL, &stmt);
>
>    If user_input happens to contain a %s then subsequent p[v]select()
> calls will expect an extra parameter to be supplied, because
> our drivers parse for % in the queries but don't recognize when
> it occurs within single-quotes.  Now, usually this would lead to
> a crash, I'd think, in p[v]select().  Not a security hole, but
> certainly unexpected behaviour.

Actually, given that the end result would be quoted, this would  
eventually (after APU DBD substitution) be interpreted as literal ?,  
$1, $2 etc. inside user input, depending on a database backend (i.e.  
it would be no parameter at all). Most likely, the user would get  
records where baz is exactly ?, $1, $2 etc. inside other text. Very  
confusing indeed.

I'm not sure why would someone want to do such a thing with prepared  
statements - it kind of defeats the purpose of prepared statements a  
bit. Usually, escaping is done for straight statements only - that's  
why I asked the question in the first place.

Anyhow, I get the point - we have the call, therefore, people may be  
tempted to use it.

>    However, in unusual cases where the coder accidentally provided
> an extra argument to p[v]select() that didn't correspond to a %s in
> the query string, and working with particular drivers, I suppose
> it might be possible for someone to make the application behave
> improperly by feeding input with %s in it.  (Maybe the %s specifiers
> and the number of arguments were in sync originally, and then a coder
> removed the last specifier without thinking to remove the last
> p[v]select() argument.)  Again, I'm not sure if it leads to a
> security issue per se, but it's not ideal.

I don't know for sure, but to be on the safe side, I think it's better  
to assume that it is possible.

>    So, my thought was: since APR DBD really encourages the use of
> placeholders and prepared statements, maybe it should discourage
> the use SQL escaping entirely.  Obviously you can't prevent people
> from building SQL queries with completely unescaped user input.
> People can always shoot themselves in the foot.  But it could be
> made as clear as possible that the API expects ASCII-based SQL
> queries with no embedded data, and all data should go into
> arguments.

You have me convinced. Simple stuff can be done with straight,  
constant strings and with query/select calls. More compilicated stuff  
should be done with p[v]query/select calls. We should not be in  
business of helping users along when they have a desire to shoot  
themselves in the foot.

At present, the only database backend that doesn't support prepared  
statements is sqlite2. So, we are relatively well covered there.

Now, the real question is: can we make apr_dbd_escape() return NULL in  
1.3 and still claim binary compatibility with 1.2?

-- 
Bojan

Mime
View raw message