Re: [Geoserver-devel] Bug with single quote in feature fields?

[Andrea A. <aa...@op...>]

Lourens Veen ha scritto:
> On Friday 09 November 2007 16:08:58 aa...@op... wrote:
>> On Fri, November 9, 2007 3:58 pm, Gabriel Roldán wrote:
>>> On Friday 09 November 2007 03:48:03 pm Gertjan van Oosten wrote:
>>>> Hi Gabriel,
>>>>
>>>> As quoted from Gabriel Roldán <gab...@gm...>:
>>>>> unfortunately it seems like a silly bug. At some point I hope
>>>>> jdbc datastores to use jdbc prepared statements
>>>> What?!?  It's been a while since I looked at the code, but do you
>>>> mean to say it's not using PreparedStatement?  Unbelievable!
>>> I went quickly through the code before stating that and that was my
>>> impression, though I might be wrong. Postgis anyone?
>> I can confirm that. Only the Oracle data store uses prepared
>> statements, and only to set the geometry (the other parameters are
>> still set in the query manually).
>>
>> There have been talks about switching to prepared statement usage but
>> since that would be a major change it would require some
>> sponsoring...
> 
> Changing the code to properly escape strings may well be achievable 
> though, and probably should be done ASAP. As Gertjan said, this sounds 
> like an SQL injection attack waiting to happen.

Agreed. Yet this has to be fixed in all databases, so it's no piece
of cake either, especially given two of them (oracle, mysql) are in 
unmantained state, the postgis mantainer is in (well deserved) vacation
and the db2 mantainer has to be contacted before any change is made.
Are you sure escaping the single quote would prevent sql injection
attacks? Here is some description of the issue (with some comments
about escaping quotes):
http://ocliteracy.com/techtips/sql-injection.html

We'll see what we can do. The problem has been known for quite
a bit of time now, the reason it hasn't been addressed it's that
fixing this in all jdbc datastores is not easy, doing that without 
introducing side effects is non trivial either (our test suites
are in place, but they do usually catch some bugs, not all conceivable 
ones), and up until a couple of months ago it would have had quite
a performance impact too (prepared statements are either cached
and reused or in most dbs take a performance hit because there is
a first round trip to the db to check the sql is correct and to
setup the plan, and then another one to actually execute the
statement. We introduced a connection pool library that can cache
prepared statements only during the summer). See also:
http://jira.codehaus.org/browse/GEOS-597
http://jira.codehaus.org/browse/GEOT-219

Cheers
Andrea