Patch applied for SQL Injection vulnerability for setObject(int, Object, int)

I have to disagree; SQL injection can happen just from input parameters, as
described. The only thing left out was the quotes. If you construct the
query as:
String query = "SELECT * from address_book WHERE name = '" + userInput +
"'";

Then the user needs to change his input to: "joe'; delete from
address_book; '"

I don't know about the JDBC driver, but perl's DBI driver would handle the
above IF it were a parameterized query by escaping all quotes in the user's
input. So if instead of constructing it by hand, you had the "WHERE name
= ?" form and the user passed in the above, postgresql would see:

SELECT * from address_book WHERE name = 'joe''; delete from address_book;
''

(I'm assuming postgresql escapes quotes by doubling them, I don't recall
for sure.)
Hopefully the JDBC driver will do this as well. If not, then all user input
needs to be scanned for quotes, semicolons, etc., so they can be properly
escaped to avoid SQL injection attacks. Incidentally, such attacks might be
a second select query instead of deleting records, so as to get info on all
users in the database instead of just themselves for instance. In that case
it would be much less obvious that an attack had occurred.

Wes Sheldahl

Dmitry Tkach <dmitry(at)openratings(dot)com>@postgresql.org on 07/17/2003 10:47:49
AM

Sent by: pgsql-jdbc-owner(at)postgresql(dot)org

To: Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk>
cc: "pgsql-jdbc @ postgresql . org" <pgsql-jdbc(at)postgresql(dot)org>
Subject: Re: [JDBC] Prepared Statements

>
> This is a security hole known as SQL injection.

No, it isn't :-)
The "hole" you are referring to is letting the users type in entire
queries, not just input parameters.
As long as you have control over how your sql is constructed, you not
any less (nor any more) safe with plain Statements than you would be
with PreparedStatements. The do the same exact thing.

Dima

> If you are using a normal Statement then your users can probably
> delete whole tables from the database but with a PreparedStatement you
> would write
>
> String query = "SELECT * from address_book WHERE name = ?"
>
> and the command actually passed over to the database would be
>
> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>
> I'm sure you can see the difference. Maybe PreparedStatements will
> have a performance gain in some future release but at the moment they
> have a vital role to play in database security.
>

---------------------------(end of broadcast)---------------------------
TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

If using a PreparedStatement the driver correctly escapes all values to
avoid SQL injection attacks. While this can also be done when using a
regular Statement object, it is then the resposibility of the programmer
to a) remember they need to escape, b) know specificially how postgresql
needs things escaped, and c) to actually escape all user input.
Invariably this will be forgotten some of the time and therefore I would
always recommend using PreparedStatements when you don't have control
over the values that are being used in the SQL statements.

thanks,
--Barry

wsheldah(at)lexmark(dot)com wrote:
> I have to disagree; SQL injection can happen just from input parameters, as
> described. The only thing left out was the quotes. If you construct the
> query as:
> String query = "SELECT * from address_book WHERE name = '" + userInput +
> "'";
>
> Then the user needs to change his input to: "joe'; delete from
> address_book; '"
>
> I don't know about the JDBC driver, but perl's DBI driver would handle the
> above IF it were a parameterized query by escaping all quotes in the user's
> input. So if instead of constructing it by hand, you had the "WHERE name
> = ?" form and the user passed in the above, postgresql would see:
>
> SELECT * from address_book WHERE name = 'joe''; delete from address_book;
> ''
>
> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
> for sure.)
> Hopefully the JDBC driver will do this as well. If not, then all user input
> needs to be scanned for quotes, semicolons, etc., so they can be properly
> escaped to avoid SQL injection attacks. Incidentally, such attacks might be
> a second select query instead of deleting records, so as to get info on all
> users in the database instead of just themselves for instance. In that case
> it would be much less obvious that an attack had occurred.
>
> Wes Sheldahl
>
>
>
> Dmitry Tkach <dmitry(at)openratings(dot)com>@postgresql.org on 07/17/2003 10:47:49
> AM
>
> Sent by: pgsql-jdbc-owner(at)postgresql(dot)org
>
>
> To: Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk>
> cc: "pgsql-jdbc @ postgresql . org" <pgsql-jdbc(at)postgresql(dot)org>
> Subject: Re: [JDBC] Prepared Statements
>
>
>
>
>>This is a security hole known as SQL injection.
>
>
> No, it isn't :-)
> The "hole" you are referring to is letting the users type in entire
> queries, not just input parameters.
> As long as you have control over how your sql is constructed, you not
> any less (nor any more) safe with plain Statements than you would be
> with PreparedStatements. The do the same exact thing.
>
> Dima
>
>
>>If you are using a normal Statement then your users can probably
>>delete whole tables from the database but with a PreparedStatement you
>>would write
>>
>>String query = "SELECT * from address_book WHERE name = ?"
>>
>>and the command actually passed over to the database would be
>>
>>SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>
>>I'm sure you can see the difference. Maybe PreparedStatements will
>>have a performance gain in some future release but at the moment they
>>have a vital role to play in database security.
>>
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

Barry Lind wrote:

> If using a PreparedStatement the driver correctly escapes all values
> to avoid SQL injection attacks.

No, it doesn't :-)
For example:

PreparedStatement s = c.prepareStatement ("select * from user where id =
?");
s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
System.out.println (s.toString ());

select * from user where id=null;drop database mydb

:-)

Dima

> While this can also be done when using a regular Statement object, it
> is then the resposibility of the programmer to a) remember they need
> to escape, b) know specificially how postgresql needs things escaped,
> and c) to actually escape all user input. Invariably this will be
> forgotten some of the time and therefore I would always recommend
> using PreparedStatements when you don't have control over the values
> that are being used in the SQL statements.
>
> thanks,
> --Barry
>
>
> wsheldah(at)lexmark(dot)com wrote:
>
>> I have to disagree; SQL injection can happen just from input
>> parameters, as
>> described. The only thing left out was the quotes. If you construct the
>> query as:
>> String query = "SELECT * from address_book WHERE name = '" + userInput +
>> "'";
>>
>> Then the user needs to change his input to: "joe'; delete from
>> address_book; '"
>>
>> I don't know about the JDBC driver, but perl's DBI driver would
>> handle the
>> above IF it were a parameterized query by escaping all quotes in the
>> user's
>> input. So if instead of constructing it by hand, you had the "WHERE name
>> = ?" form and the user passed in the above, postgresql would see:
>>
>> SELECT * from address_book WHERE name = 'joe''; delete from
>> address_book;
>> ''
>>
>> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
>> for sure.)
>> Hopefully the JDBC driver will do this as well. If not, then all user
>> input
>> needs to be scanned for quotes, semicolons, etc., so they can be
>> properly
>> escaped to avoid SQL injection attacks. Incidentally, such attacks
>> might be
>> a second select query instead of deleting records, so as to get info
>> on all
>> users in the database instead of just themselves for instance. In
>> that case
>> it would be much less obvious that an attack had occurred.
>>
>> Wes Sheldahl
>>
>>
>>
>> Dmitry Tkach <dmitry(at)openratings(dot)com>@postgresql.org on 07/17/2003
>> 10:47:49
>> AM
>>
>> Sent by: pgsql-jdbc-owner(at)postgresql(dot)org
>>
>>
>> To: Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk>
>> cc: "pgsql-jdbc @ postgresql . org" <pgsql-jdbc(at)postgresql(dot)org>
>> Subject: Re: [JDBC] Prepared Statements
>>
>>
>>
>>
>>> This is a security hole known as SQL injection.
>>
>>
>>
>> No, it isn't :-)
>> The "hole" you are referring to is letting the users type in entire
>> queries, not just input parameters.
>> As long as you have control over how your sql is constructed, you not
>> any less (nor any more) safe with plain Statements than you would be
>> with PreparedStatements. The do the same exact thing.
>>
>> Dima
>>
>>
>>> If you are using a normal Statement then your users can probably
>>> delete whole tables from the database but with a PreparedStatement you
>>> would write
>>>
>>> String query = "SELECT * from address_book WHERE name = ?"
>>>
>>> and the command actually passed over to the database would be
>>>
>>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>>
>>> I'm sure you can see the difference. Maybe PreparedStatements will
>>> have a performance gain in some future release but at the moment they
>>> have a vital role to play in database security.
>>>
>>
>>
>>
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 1: subscribe and unsubscribe commands go to
>> majordomo(at)postgresql(dot)org
>>
>>
>>
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>
>
>

This looks like a bug to me.
Still, once these kind of bugs are identified and fixed, it's much safer
to rely on the prepared statement's security than on your own escaping.
Besides, if you would set the parameter as string (as you would normally
do in an application) and not as an integer, the escaping would be
correct. And numeric parameters normally get checked for valid
formatting in the application, so this would not slip through.

Cheers,
Csasba.

On Fri, 2003-07-18 at 16:18, Dmitry Tkach wrote:
> Barry Lind wrote:
>
> > If using a PreparedStatement the driver correctly escapes all values
> > to avoid SQL injection attacks.
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>
> Dima
>
>
> > While this can also be done when using a regular Statement object, it
> > is then the resposibility of the programmer to a) remember they need
> > to escape, b) know specificially how postgresql needs things escaped,
> > and c) to actually escape all user input. Invariably this will be
> > forgotten some of the time and therefore I would always recommend
> > using PreparedStatements when you don't have control over the values
> > that are being used in the SQL statements.
> >
> > thanks,
> > --Barry
> >
> >
> > wsheldah(at)lexmark(dot)com wrote:
> >
> >> I have to disagree; SQL injection can happen just from input
> >> parameters, as
> >> described. The only thing left out was the quotes. If you construct the
> >> query as:
> >> String query = "SELECT * from address_book WHERE name = '" + userInput +
> >> "'";
> >>
> >> Then the user needs to change his input to: "joe'; delete from
> >> address_book; '"
> >>
> >> I don't know about the JDBC driver, but perl's DBI driver would
> >> handle the
> >> above IF it were a parameterized query by escaping all quotes in the
> >> user's
> >> input. So if instead of constructing it by hand, you had the "WHERE name
> >> = ?" form and the user passed in the above, postgresql would see:
> >>
> >> SELECT * from address_book WHERE name = 'joe''; delete from
> >> address_book;
> >> ''
> >>
> >> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
> >> for sure.)
> >> Hopefully the JDBC driver will do this as well. If not, then all user
> >> input
> >> needs to be scanned for quotes, semicolons, etc., so they can be
> >> properly
> >> escaped to avoid SQL injection attacks. Incidentally, such attacks
> >> might be
> >> a second select query instead of deleting records, so as to get info
> >> on all
> >> users in the database instead of just themselves for instance. In
> >> that case
> >> it would be much less obvious that an attack had occurred.
> >>
> >> Wes Sheldahl
> >>
> >>
> >>
> >> Dmitry Tkach <dmitry(at)openratings(dot)com>@postgresql.org on 07/17/2003
> >> 10:47:49
> >> AM
> >>
> >> Sent by: pgsql-jdbc-owner(at)postgresql(dot)org
> >>
> >>
> >> To: Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk>
> >> cc: "pgsql-jdbc @ postgresql . org" <pgsql-jdbc(at)postgresql(dot)org>
> >> Subject: Re: [JDBC] Prepared Statements
> >>
> >>
> >>
> >>
> >>> This is a security hole known as SQL injection.
> >>
> >>
> >>
> >> No, it isn't :-)
> >> The "hole" you are referring to is letting the users type in entire
> >> queries, not just input parameters.
> >> As long as you have control over how your sql is constructed, you not
> >> any less (nor any more) safe with plain Statements than you would be
> >> with PreparedStatements. The do the same exact thing.
> >>
> >> Dima
> >>
> >>
> >>> If you are using a normal Statement then your users can probably
> >>> delete whole tables from the database but with a PreparedStatement you
> >>> would write
> >>>
> >>> String query = "SELECT * from address_book WHERE name = ?"
> >>>
> >>> and the command actually passed over to the database would be
> >>>
> >>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
> >>>
> >>> I'm sure you can see the difference. Maybe PreparedStatements will
> >>> have a performance gain in some future release but at the moment they
> >>> have a vital role to play in database security.
> >>>
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------(end of broadcast)---------------------------
> >> TIP 1: subscribe and unsubscribe commands go to
> >> majordomo(at)postgresql(dot)org
> >>
> >>
> >>
> >>
> >>
> >> ---------------------------(end of broadcast)---------------------------
> >> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
> >>
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

Dmitry Tkach wrote:
> Barry Lind wrote:
>
>> If using a PreparedStatement the driver correctly escapes all values
>> to avoid SQL injection attacks.
>
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>

I don't believe this is actually being sent to the backend, maybe it is
just a toString() bug.

The backend should get:

select * from user where id='null;drop database mydb'

(If it does not it is a bug.)

P.S.: The example case would only succeed if the DBA is an idiot.
You program should not be accessing the database (for this queries at
least) as an user who can drop databases unless it is a privileged
program for privileged users (who could do the damage using plain psql
anyway). Perhaps the injection of a 'DELETE FROM mytable' would be a
more realistic example.

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

I have checked, the query is indeed sent like that to the backend, I've
just checked.
It is a bug.
Presumably for number types the parameter set is passed as it is,
without any escaping.

Cheers,
Csaba.

On Fri, 2003-07-18 at 16:38, Fernando Nasser wrote:
> Dmitry Tkach wrote:
> > Barry Lind wrote:
> >
> >> If using a PreparedStatement the driver correctly escapes all values
> >> to avoid SQL injection attacks.
> >
> >
> > No, it doesn't :-)
> > For example:
> >
> > PreparedStatement s = c.prepareStatement ("select * from user where id =
> > ?");
> > s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> > System.out.println (s.toString ());
> >
> > select * from user where id=null;drop database mydb
> >
> > :-)
> >
>
> I don't believe this is actually being sent to the backend, maybe it is
> just a toString() bug.
>
> The backend should get:
>
> select * from user where id='null;drop database mydb'
>
> (If it does not it is a bug.)
>
>
> P.S.: The example case would only succeed if the DBA is an idiot.
> You program should not be accessing the database (for this queries at
> least) as an user who can drop databases unless it is a privileged
> program for privileged users (who could do the damage using plain psql
> anyway). Perhaps the injection of a 'DELETE FROM mytable' would be a
> more realistic example.
>
>
> --
> Fernando Nasser
> Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario M4P 2C9
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 8: explain analyze is your friend
>

Csaba Nagy wrote:
> This looks like a bug to me.
> Still, once these kind of bugs are identified and fixed,

That is why I like open source software, right there.

Erik

Fernando Nasser wrote:

>>
>
> I don't believe this is actually being sent to the backend, maybe it
> is just a toString() bug.

You better do believe it. I tried it, and it works. :-)

>
> The backend should get:
>
> select * from user where id='null;drop database mydb'
>
> (If it does not it is a bug.)

Nah... That's what it would get if you did setString()... setObject ()
doesn't work that way.
I tend to agree, it's a bug - if the type is INTEGER, it should be
checking if the object, passed in is really numeric.

The thing is that, at least, in the current state of the driver, this is
a *really nice* bug, that gives you the only way to use certain
functionality....
For example:

PreparedStatement stmt = c.prepareStatement ("select * from mytable
where data in ?");
stmt.setObject (1, "(1,2,3,4,5)", Types.INTEGER);

... if the "bug" was fixed, there would be no other way to do this kind
of thing :-(

>
>
>
> P.S.: The example case would only succeed if the DBA is an idiot.

No objection here :-)
But, in my opinion, the same comment applies to all the earlier examples
(without PreparedStatements) just as well - the point is, if you are an
idiot, you will trash your database one way or another, with or without
using PS, and if you are not, then you won't :-)

>
> You program should not be accessing the database (for this queries at
> least) as an user who can drop databases unless it is a privileged
> program for privileged users (who could do the damage using plain psql
> anyway). Perhaps the injection of a 'DELETE FROM mytable' would be a
> more realistic example.

Come on!... Replace 'drop databse' with just 'do whatever you want' :-)
I just put it in to make it look scarier :-) That was a joke ...
It's just an illustration of the nice 'injection attac' using
PreparedStatements, that everybody else around seems to believe is
impossible.

It isn't. If the person writing the code is an idiot, PreparedStatements
won't help him (nothing will), and if he isn't they won't help him
either (because he wouldn't need that kind of help).
I would like the performance benefit of PS (if there was any)... But
security? No way...
If you accept any kind of user input and send it to the database without
bothering to check what the hell is there, you will be doomed, and no
PreparedStatement in the world will save you :-)

Dima

Dmitry Tkach wrote:
> Fernando Nasser wrote:
>
>>>
>>
>> I don't believe this is actually being sent to the backend, maybe it
>> is just a toString() bug.
>
>
> You better do believe it. I tried it, and it works. :-)
>
>>
>> The backend should get:
>>
>> select * from user where id='null;drop database mydb'
>>
>> (If it does not it is a bug.)
>
>
> Nah... That's what it would get if you did setString()... setObject ()
> doesn't work that way.
> I tend to agree, it's a bug - if the type is INTEGER, it should be
> checking if the object, passed in is really numeric.
>
> The thing is that, at least, in the current state of the driver, this is
> a *really nice* bug, that gives you the only way to use certain
> functionality....
> For example:
>
> PreparedStatement stmt = c.prepareStatement ("select * from mytable
> where data in ?");
> stmt.setObject (1, "(1,2,3,4,5)", Types.INTEGER);
>
> ... if the "bug" was fixed, there would be no other way to do this kind
> of thing :-(
>

Well, I guess the bug will have be fixed asap as it is a security risk.

What is the proper JDBC way for filling IN lists in prepared statements?

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

>
> Well, I guess the bug will have be fixed asap as it is a security risk.
>
> What is the proper JDBC way for filling IN lists in prepared statements?
>

I'm no JDBC expert, but the way we do it: create a prepared statement
with 100 (or whatever the max nr. of accepted params is) parameter
placeholders, and set the ones which are actually needed to their
parameter values, and set the rest to null.
The nulls will be finally ignored by the database.
Not the best solution, but it works just fine for us.

Cheers,
Csaba.

>
> Well, I guess the bug will have be fixed asap as it is a security risk.

I am afraid, it will :-(
That's exactly why, as I told you yesterday, I tried to avoid upgrading
my driver versions too frequently - because of the 'bug fixes' like
this, that break stuff....

>
> What is the proper JDBC way for filling IN lists in prepared statements?

I am afraid, there is no standard about it :-(
Depends on the vendor...
Most of them (not postgres) support SQLData - to let you define and pass
in arbitrary types...

Some (like infomirx for example... don't know about Oracle) have sets
and arrays interchangeable - so that
setObject (1, sqlArrayContainingIdsToMatch, Types.ARRAY)
can be used...

Some people are used to hacks, like one described in an earlier post -
where yuo create a statement with an awful lot of questionmarks, and
then set each member of the set separately and cross your fingers,
hoping that you have enough placeholders for your whole set...

Dima

That's a hack indeed, but there's no finger crossing :-)
We do chunk our queries if needed, i.e. execute them multiple times if
the parameter set is too big. We mostly use this technique to filter a
set according to some criteria. Now if the set is too big, we do the
filtering chunk-wise, and then cumulate the result. I admit that it
won't work for all situations, but then if you have a big set, you might
be looking for a different solution like dumping them in to a temporary
table.

Cheers,
Csaba.

> Some people are used to hacks, like one described in an earlier post -
> where yuo create a statement with an awful lot of questionmarks, and
> then set each member of the set separately and cross your fingers,
> hoping that you have enough placeholders for your whole set...
>
> Dima
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings
>

Dmitry,

That is a bug. Thanks for pointing it out. Anyone care to submit a patch?

--Barry

Dmitry Tkach wrote:
> Barry Lind wrote:
>
>> If using a PreparedStatement the driver correctly escapes all values
>> to avoid SQL injection attacks.
>
>
> No, it doesn't :-)
> For example:
>
> PreparedStatement s = c.prepareStatement ("select * from user where id =
> ?");
> s.setObject (1, "null;drop database mydatabase", Types.INTEGER);
> System.out.println (s.toString ());
>
> select * from user where id=null;drop database mydb
>
> :-)
>
> Dima
>
>
>> While this can also be done when using a regular Statement object, it
>> is then the resposibility of the programmer to a) remember they need
>> to escape, b) know specificially how postgresql needs things escaped,
>> and c) to actually escape all user input. Invariably this will be
>> forgotten some of the time and therefore I would always recommend
>> using PreparedStatements when you don't have control over the values
>> that are being used in the SQL statements.
>>
>> thanks,
>> --Barry
>>
>>
>> wsheldah(at)lexmark(dot)com wrote:
>>
>>> I have to disagree; SQL injection can happen just from input
>>> parameters, as
>>> described. The only thing left out was the quotes. If you construct the
>>> query as:
>>> String query = "SELECT * from address_book WHERE name = '" + userInput +
>>> "'";
>>>
>>> Then the user needs to change his input to: "joe'; delete from
>>> address_book; '"
>>>
>>> I don't know about the JDBC driver, but perl's DBI driver would
>>> handle the
>>> above IF it were a parameterized query by escaping all quotes in the
>>> user's
>>> input. So if instead of constructing it by hand, you had the "WHERE name
>>> = ?" form and the user passed in the above, postgresql would see:
>>>
>>> SELECT * from address_book WHERE name = 'joe''; delete from
>>> address_book;
>>> ''
>>>
>>> (I'm assuming postgresql escapes quotes by doubling them, I don't recall
>>> for sure.)
>>> Hopefully the JDBC driver will do this as well. If not, then all user
>>> input
>>> needs to be scanned for quotes, semicolons, etc., so they can be
>>> properly
>>> escaped to avoid SQL injection attacks. Incidentally, such attacks
>>> might be
>>> a second select query instead of deleting records, so as to get info
>>> on all
>>> users in the database instead of just themselves for instance. In
>>> that case
>>> it would be much less obvious that an attack had occurred.
>>>
>>> Wes Sheldahl
>>>
>>>
>>>
>>> Dmitry Tkach <dmitry(at)openratings(dot)com>@postgresql.org on 07/17/2003
>>> 10:47:49
>>> AM
>>>
>>> Sent by: pgsql-jdbc-owner(at)postgresql(dot)org
>>>
>>>
>>> To: Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk>
>>> cc: "pgsql-jdbc @ postgresql . org" <pgsql-jdbc(at)postgresql(dot)org>
>>> Subject: Re: [JDBC] Prepared Statements
>>>
>>>
>>>
>>>
>>>> This is a security hole known as SQL injection.
>>>
>>>
>>>
>>>
>>> No, it isn't :-)
>>> The "hole" you are referring to is letting the users type in entire
>>> queries, not just input parameters.
>>> As long as you have control over how your sql is constructed, you not
>>> any less (nor any more) safe with plain Statements than you would be
>>> with PreparedStatements. The do the same exact thing.
>>>
>>> Dima
>>>
>>>
>>>> If you are using a normal Statement then your users can probably
>>>> delete whole tables from the database but with a PreparedStatement you
>>>> would write
>>>>
>>>> String query = "SELECT * from address_book WHERE name = ?"
>>>>
>>>> and the command actually passed over to the database would be
>>>>
>>>> SELECT * from address_book WHERE name = 'joe;delete from address_book'
>>>>
>>>> I'm sure you can see the difference. Maybe PreparedStatements will
>>>> have a performance gain in some future release but at the moment they
>>>> have a vital role to play in database security.
>>>>
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------(end of broadcast)---------------------------
>>> TIP 1: subscribe and unsubscribe commands go to
>>> majordomo(at)postgresql(dot)org
>>>
>>>
>>>
>>>
>>>
>>> ---------------------------(end of broadcast)---------------------------
>>> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>>
>>
>>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

In these cases, I just set a single question mark in the query... then I use setObject(index, parameters, Types.NUMERIC)
In the "parameters" variable I pass the values concatenated, like:

PreparedStatement prep = conn.preparePreparedStatement("SELECT * FROM foo WHERE bar IN (?)");
prep.setObject(1, "1, 2, 3", Types.NUMERIC);

The problem about this technique is that I can't use driver's scaping of Strings... I just hope this keeps working in future versions of the driver :-)
There is a way that I can cann driver's scaping methods? Would be nice if they were public.

On 18 Jul 2003 17:32:34 +0200
Csaba Nagy <nagy(at)ecircle-ag(dot)com> wrote:

> >
> > Well, I guess the bug will have be fixed asap as it is a security risk.
> >
> > What is the proper JDBC way for filling IN lists in prepared statements?
> >
>
> I'm no JDBC expert, but the way we do it: create a prepared statement
> with 100 (or whatever the max nr. of accepted params is) parameter
> placeholders, and set the ones which are actually needed to their
> parameter values, and set the rest to null.
> The nulls will be finally ignored by the database.
> Not the best solution, but it works just fine for us.
>
> Cheers,
> Csaba.
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

Barry Lind wrote:
> Dmitry,
>
> That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
>

Kim's patch fixes this. It is pending approval.

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

To speed things up a bit, since the regoutParam patch is not likely to
be approved anytime soon.

This patch
- adds single quotes for numbers in setObject and also setInt/Byte/etc.
- Improves getInt/Long when you may have parser errors if you're too
close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
- Improves radix point handling when using setObject to an integer
parameter while passing in a float. This is especially important in
callable statements.

Cheers,

Kim

On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
> Barry Lind wrote:
> > Dmitry,
> >
> > That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
> >
>
> Kim's patch fixes this. It is pending approval.
>
>
>
> --
> Fernando Nasser
> Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
> 2323 Yonge Street, Suite #300
> Toronto, Ontario M4P 2C9
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

Hey, this means I will not be able anymore to use setObject() to set my IN values like I did??

On Fri, 18 Jul 2003 13:27:13 -0400
Dmitry Tkach <dmitry(at)openratings(dot)com> wrote:

> Kim Ho wrote:
>
> >To speed things up a bit, since the regoutParam patch is not likely to
> >be approved anytime soon.
> >
> >This patch
> >- adds single quotes for numbers in setObject and also setInt/Byte/etc.
> >- Improves getInt/Long when you may have parser errors if you're too
> >close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
> >- Improves radix point handling when using setObject to an integer
> >parameter while passing in a float. This is especially important in
> >callable statements.
> >
> I see :-)
> Aside from taking away that ability to be able to pass sets using
> setObject(), which is unfortunate, about the only improvement this makes
> seems to be that the malicious "injector" would have to pass in a string
> like (just making sure it doesn't contain any dots :-)
>
> 1';delete from precious_table where 'true
>
> to make a statement like
>
> select * from somewhere where id=?
>
> to get sent as "select * from somewhere where id='1';delete from
> precious_table where 'true'" and wipe out your precious table :-)
>
>
> You really believe you can win this race, by plugging this particular
> hole, I am afraid, you are going to have to always parse the input
> that,s supposed to be numerical into a number...
>
>
> Dima
>
> P.S. On a different note, something like
> "select ?"
> setString (1, "\047");
>
> returns "\047" when executed. Now *this*, is a bug - because it is
> supposed to return a string, containing a quote as a single character...
>
>
>
>
>
>
> >
> >Cheers,
> >
> >Kim
> >
> >On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
> >
> >
> >>Barry Lind wrote:
> >>
> >>
> >>>Dmitry,
> >>>
> >>>That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
> >>>
> >>>
> >>>
> >>Kim's patch fixes this. It is pending approval.
> >>
> >>
> >>
> >>--
> >>Fernando Nasser
> >>Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
> >>2323 Yonge Street, Suite #300
> >>Toronto, Ontario M4P 2C9
> >>
> >>
> >>---------------------------(end of broadcast)---------------------------
> >>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
> >>
> >>
> >
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >? temp.diff
> >Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> >===================================================================
> >RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> >retrieving revision 1.13
> >diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> >*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13
> >--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 18 Jul 2003 17:02:20 -0000
> >*************** public abstract class AbstractJdbc1Resul
> >*** 805,811 ****
> > try
> > {
> > s = s.trim();
> >! return Integer.parseInt(s);
> > }
> > catch (NumberFormatException e)
> > {
> >--- 805,811 ----
> > try
> > {
> > s = s.trim();
> >! return Float.valueOf(s).intValue();
> > }
> > catch (NumberFormatException e)
> > {
> >*************** public abstract class AbstractJdbc1Resul
> >*** 822,828 ****
> > try
> > {
> > s = s.trim();
> >! return Long.parseLong(s);
> > }
> > catch (NumberFormatException e)
> > {
> >--- 822,828 ----
> > try
> > {
> > s = s.trim();
> >! return Double.valueOf(s).longValue();
> > }
> > catch (NumberFormatException e)
> > {
> >Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> >===================================================================
> >RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> >retrieving revision 1.27
> >diff -c -p -r1.27 AbstractJdbc1Statement.java
> >*** org/postgresql/jdbc1/AbstractJdbc1Statement.java 9 Jul 2003 05:12:04 -0000 1.27
> >--- org/postgresql/jdbc1/AbstractJdbc1Statement.java 18 Jul 2003 17:02:22 -0000
> >*************** public abstract class AbstractJdbc1State
> >*** 920,926 ****
> > */
> > public void setByte(int parameterIndex, byte x) throws SQLException
> > {
> >! bind(parameterIndex, Integer.toString(x), PG_TEXT);
> > }
> >
> > /*
> >--- 920,926 ----
> > */
> > public void setByte(int parameterIndex, byte x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 933,939 ****
> > */
> > public void setShort(int parameterIndex, short x) throws SQLException
> > {
> >! bind(parameterIndex, Integer.toString(x), PG_INT2);
> > }
> >
> > /*
> >--- 933,939 ----
> > */
> > public void setShort(int parameterIndex, short x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 946,952 ****
> > */
> > public void setInt(int parameterIndex, int x) throws SQLException
> > {
> >! bind(parameterIndex, Integer.toString(x), PG_INTEGER);
> > }
> >
> > /*
> >--- 946,952 ----
> > */
> > public void setInt(int parameterIndex, int x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 959,965 ****
> > */
> > public void setLong(int parameterIndex, long x) throws SQLException
> > {
> >! bind(parameterIndex, Long.toString(x), PG_INT8);
> > }
> >
> > /*
> >--- 959,965 ----
> > */
> > public void setLong(int parameterIndex, long x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 972,978 ****
> > */
> > public void setFloat(int parameterIndex, float x) throws SQLException
> > {
> >! bind(parameterIndex, Float.toString(x), PG_FLOAT);
> > }
> >
> > /*
> >--- 972,978 ----
> > */
> > public void setFloat(int parameterIndex, float x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 985,991 ****
> > */
> > public void setDouble(int parameterIndex, double x) throws SQLException
> > {
> >! bind(parameterIndex, Double.toString(x), PG_DOUBLE);
> > }
> >
> > /*
> >--- 985,991 ----
> > */
> > public void setDouble(int parameterIndex, double x) throws SQLException
> > {
> >! bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
> > }
> >
> > /*
> >*************** public abstract class AbstractJdbc1State
> >*** 1003,1009 ****
> > setNull(parameterIndex, Types.DECIMAL);
> > else
> > {
> >! bind(parameterIndex, x.toString(), PG_NUMERIC);
> > }
> > }
> >
> >--- 1003,1009 ----
> > setNull(parameterIndex, Types.DECIMAL);
> > else
> > {
> >! bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
> > }
> > }
> >
> >*************** public abstract class AbstractJdbc1State
> >*** 1464,1486 ****
> > switch (targetSqlType)
> > {
> > case Types.INTEGER:
> >- if (x instanceof Boolean)
> >- bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> >- else
> >- bind(parameterIndex, x.toString(), PG_INTEGER);
> >- break;
> > case Types.TINYINT:
> > case Types.SMALLINT:
> > case Types.BIGINT:
> > case Types.REAL:
> > case Types.FLOAT:
> > case Types.DOUBLE:
> > case Types.DECIMAL:
> > case Types.NUMERIC:
> >! if (x instanceof Boolean)
> >! bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> >! else
> >! bind(parameterIndex, x.toString(), PG_NUMERIC);
> > break;
> > case Types.CHAR:
> > case Types.VARCHAR:
> >--- 1464,1484 ----
> > switch (targetSqlType)
> > {
> > case Types.INTEGER:
> > case Types.TINYINT:
> > case Types.SMALLINT:
> >+ x = removeRadix(x,Types.INTEGER);
> >+ bindNumber(parameterIndex,x,PG_INTEGER);
> >+ break;
> > case Types.BIGINT:
> >+ x = removeRadix(x,Types.BIGINT);
> >+ bindNumber(parameterIndex,x,PG_INT8);
> >+ break;
> > case Types.REAL:
> > case Types.FLOAT:
> > case Types.DOUBLE:
> > case Types.DECIMAL:
> > case Types.NUMERIC:
> >! bindNumber(parameterIndex,x,PG_NUMERIC);
> > break;
> > case Types.CHAR:
> > case Types.VARCHAR:
> >*************** public abstract class AbstractJdbc1State
> >*** 2026,2031 ****
> >--- 2024,2056 ----
> > if (parameterIndex != 1)
> > throw new PSQLException("postgresql.call.noinout");
> > }
> >+
> >+ private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> >+ {
> >+ if (x instanceof Boolean)
> >+ bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> >+ else
> >+ bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> >+ }
> >+
> >+
> >+ private Object removeRadix(Object x, int sqlType)
> >+ {
> >+ if (x.toString().indexOf(".")>0)
> >+ {
> >+ switch (sqlType)
> >+ {
> >+ case Types.BIGINT:
> >+ x = String.valueOf(Double.valueOf(x.toString()).longValue());
> >+ break;
> >+ default:
> >+ x = String.valueOf(Float.valueOf(x.toString()).intValue());
> >+ break;
> >+ }
> >+ }
> >+ return x;
> >+ }
> >+
> >
> >
> >
> >
> >
> >------------------------------------------------------------------------
> >
> >
> >---------------------------(end of broadcast)---------------------------
> >TIP 3: if posting/reading through Usenet, please send an appropriate
> > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > message can get through to the mailing list cleanly
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

Kim Ho wrote:

>To speed things up a bit, since the regoutParam patch is not likely to
>be approved anytime soon.
>
>This patch
>- adds single quotes for numbers in setObject and also setInt/Byte/etc.
>- Improves getInt/Long when you may have parser errors if you're too
>close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
>- Improves radix point handling when using setObject to an integer
>parameter while passing in a float. This is especially important in
>callable statements.
>
I see :-)
Aside from taking away that ability to be able to pass sets using
setObject(), which is unfortunate, about the only improvement this makes
seems to be that the malicious "injector" would have to pass in a string
like (just making sure it doesn't contain any dots :-)

1';delete from precious_table where 'true

to make a statement like

select * from somewhere where id=?

to get sent as "select * from somewhere where id='1';delete from
precious_table where 'true'" and wipe out your precious table :-)

You really believe you can win this race, by plugging this particular
hole, I am afraid, you are going to have to always parse the input
that,s supposed to be numerical into a number...

Dima

P.S. On a different note, something like
"select ?"
setString (1, "\047");

returns "\047" when executed. Now *this*, is a bug - because it is
supposed to return a string, containing a quote as a single character...

>
>Cheers,
>
>Kim
>
>On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>
>>Barry Lind wrote:
>>
>>
>>>Dmitry,
>>>
>>>That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
>>>
>>>
>>>
>>Kim's patch fixes this. It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>
>>
>
>
>
>
>------------------------------------------------------------------------
>
>? temp.diff
>Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
>retrieving revision 1.13
>diff -c -p -r1.13 AbstractJdbc1ResultSet.java
>*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13
>--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 18 Jul 2003 17:02:20 -0000
>*************** public abstract class AbstractJdbc1Resul
>*** 805,811 ****
> try
> {
> s = s.trim();
>! return Integer.parseInt(s);
> }
> catch (NumberFormatException e)
> {
>--- 805,811 ----
> try
> {
> s = s.trim();
>! return Float.valueOf(s).intValue();
> }
> catch (NumberFormatException e)
> {
>*************** public abstract class AbstractJdbc1Resul
>*** 822,828 ****
> try
> {
> s = s.trim();
>! return Long.parseLong(s);
> }
> catch (NumberFormatException e)
> {
>--- 822,828 ----
> try
> {
> s = s.trim();
>! return Double.valueOf(s).longValue();
> }
> catch (NumberFormatException e)
> {
>Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
>retrieving revision 1.27
>diff -c -p -r1.27 AbstractJdbc1Statement.java
>*** org/postgresql/jdbc1/AbstractJdbc1Statement.java 9 Jul 2003 05:12:04 -0000 1.27
>--- org/postgresql/jdbc1/AbstractJdbc1Statement.java 18 Jul 2003 17:02:22 -0000
>*************** public abstract class AbstractJdbc1State
>*** 920,926 ****
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_TEXT);
> }
>
> /*
>--- 920,926 ----
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 933,939 ****
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_INT2);
> }
>
> /*
>--- 933,939 ----
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 946,952 ****
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_INTEGER);
> }
>
> /*
>--- 946,952 ----
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 959,965 ****
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
>! bind(parameterIndex, Long.toString(x), PG_INT8);
> }
>
> /*
>--- 959,965 ----
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
>! bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 972,978 ****
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
>! bind(parameterIndex, Float.toString(x), PG_FLOAT);
> }
>
> /*
>--- 972,978 ----
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
>! bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 985,991 ****
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
>! bind(parameterIndex, Double.toString(x), PG_DOUBLE);
> }
>
> /*
>--- 985,991 ----
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
>! bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 1003,1009 ****
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
>! bind(parameterIndex, x.toString(), PG_NUMERIC);
> }
> }
>
>--- 1003,1009 ----
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
>! bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
> }
> }
>
>*************** public abstract class AbstractJdbc1State
>*** 1464,1486 ****
> switch (targetSqlType)
> {
> case Types.INTEGER:
>- if (x instanceof Boolean)
>- bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
>- else
>- bind(parameterIndex, x.toString(), PG_INTEGER);
>- break;
> case Types.TINYINT:
> case Types.SMALLINT:
> case Types.BIGINT:
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
>! if (x instanceof Boolean)
>! bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
>! else
>! bind(parameterIndex, x.toString(), PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
>--- 1464,1484 ----
> switch (targetSqlType)
> {
> case Types.INTEGER:
> case Types.TINYINT:
> case Types.SMALLINT:
>+ x = removeRadix(x,Types.INTEGER);
>+ bindNumber(parameterIndex,x,PG_INTEGER);
>+ break;
> case Types.BIGINT:
>+ x = removeRadix(x,Types.BIGINT);
>+ bindNumber(parameterIndex,x,PG_INT8);
>+ break;
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
>! bindNumber(parameterIndex,x,PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
>*************** public abstract class AbstractJdbc1State
>*** 2026,2031 ****
>--- 2024,2056 ----
> if (parameterIndex != 1)
> throw new PSQLException("postgresql.call.noinout");
> }
>+
>+ private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
>+ {
>+ if (x instanceof Boolean)
>+ bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
>+ else
>+ bind(parameterIndex, "'"+x.toString()+"'", pgtype);
>+ }
>+
>+
>+ private Object removeRadix(Object x, int sqlType)
>+ {
>+ if (x.toString().indexOf(".")>0)
>+ {
>+ switch (sqlType)
>+ {
>+ case Types.BIGINT:
>+ x = String.valueOf(Double.valueOf(x.toString()).longValue());
>+ break;
>+ default:
>+ x = String.valueOf(Float.valueOf(x.toString()).intValue());
>+ break;
>+ }
>+ }
>+ return x;
>+ }
>+
>
>
>
>
>
>------------------------------------------------------------------------
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>
>

And also, this breaks the hexademical notation:
PreparedStatement s = c.prepareStatement ("select * from mytable where
id=?");
s.setObject (1, "x'a'");

That works in the current driver will stop working after this patch is
applied.

Dima

Kim Ho wrote:

>To speed things up a bit, since the regoutParam patch is not likely to
>be approved anytime soon.
>
>This patch
>- adds single quotes for numbers in setObject and also setInt/Byte/etc.
>- Improves getInt/Long when you may have parser errors if you're too
>close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
>- Improves radix point handling when using setObject to an integer
>parameter while passing in a float. This is especially important in
>callable statements.
>
>Cheers,
>
>Kim
>
>On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>
>>Barry Lind wrote:
>>
>>
>>>Dmitry,
>>>
>>>That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
>>>
>>>
>>>
>>Kim's patch fixes this. It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>>
>>
>
>
>
>
>------------------------------------------------------------------------
>
>? temp.diff
>Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
>retrieving revision 1.13
>diff -c -p -r1.13 AbstractJdbc1ResultSet.java
>*** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13
>--- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 18 Jul 2003 17:02:20 -0000
>*************** public abstract class AbstractJdbc1Resul
>*** 805,811 ****
> try
> {
> s = s.trim();
>! return Integer.parseInt(s);
> }
> catch (NumberFormatException e)
> {
>--- 805,811 ----
> try
> {
> s = s.trim();
>! return Float.valueOf(s).intValue();
> }
> catch (NumberFormatException e)
> {
>*************** public abstract class AbstractJdbc1Resul
>*** 822,828 ****
> try
> {
> s = s.trim();
>! return Long.parseLong(s);
> }
> catch (NumberFormatException e)
> {
>--- 822,828 ----
> try
> {
> s = s.trim();
>! return Double.valueOf(s).longValue();
> }
> catch (NumberFormatException e)
> {
>Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
>===================================================================
>RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
>retrieving revision 1.27
>diff -c -p -r1.27 AbstractJdbc1Statement.java
>*** org/postgresql/jdbc1/AbstractJdbc1Statement.java 9 Jul 2003 05:12:04 -0000 1.27
>--- org/postgresql/jdbc1/AbstractJdbc1Statement.java 18 Jul 2003 17:02:22 -0000
>*************** public abstract class AbstractJdbc1State
>*** 920,926 ****
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_TEXT);
> }
>
> /*
>--- 920,926 ----
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 933,939 ****
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_INT2);
> }
>
> /*
>--- 933,939 ----
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 946,952 ****
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
>! bind(parameterIndex, Integer.toString(x), PG_INTEGER);
> }
>
> /*
>--- 946,952 ----
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
>! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 959,965 ****
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
>! bind(parameterIndex, Long.toString(x), PG_INT8);
> }
>
> /*
>--- 959,965 ----
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
>! bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 972,978 ****
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
>! bind(parameterIndex, Float.toString(x), PG_FLOAT);
> }
>
> /*
>--- 972,978 ----
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
>! bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 985,991 ****
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
>! bind(parameterIndex, Double.toString(x), PG_DOUBLE);
> }
>
> /*
>--- 985,991 ----
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
>! bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
> }
>
> /*
>*************** public abstract class AbstractJdbc1State
>*** 1003,1009 ****
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
>! bind(parameterIndex, x.toString(), PG_NUMERIC);
> }
> }
>
>--- 1003,1009 ----
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
>! bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
> }
> }
>
>*************** public abstract class AbstractJdbc1State
>*** 1464,1486 ****
> switch (targetSqlType)
> {
> case Types.INTEGER:
>- if (x instanceof Boolean)
>- bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
>- else
>- bind(parameterIndex, x.toString(), PG_INTEGER);
>- break;
> case Types.TINYINT:
> case Types.SMALLINT:
> case Types.BIGINT:
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
>! if (x instanceof Boolean)
>! bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
>! else
>! bind(parameterIndex, x.toString(), PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
>--- 1464,1484 ----
> switch (targetSqlType)
> {
> case Types.INTEGER:
> case Types.TINYINT:
> case Types.SMALLINT:
>+ x = removeRadix(x,Types.INTEGER);
>+ bindNumber(parameterIndex,x,PG_INTEGER);
>+ break;
> case Types.BIGINT:
>+ x = removeRadix(x,Types.BIGINT);
>+ bindNumber(parameterIndex,x,PG_INT8);
>+ break;
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
>! bindNumber(parameterIndex,x,PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
>*************** public abstract class AbstractJdbc1State
>*** 2026,2031 ****
>--- 2024,2056 ----
> if (parameterIndex != 1)
> throw new PSQLException("postgresql.call.noinout");
> }
>+
>+ private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
>+ {
>+ if (x instanceof Boolean)
>+ bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
>+ else
>+ bind(parameterIndex, "'"+x.toString()+"'", pgtype);
>+ }
>+
>+
>+ private Object removeRadix(Object x, int sqlType)
>+ {
>+ if (x.toString().indexOf(".")>0)
>+ {
>+ switch (sqlType)
>+ {
>+ case Types.BIGINT:
>+ x = String.valueOf(Double.valueOf(x.toString()).longValue());
>+ break;
>+ default:
>+ x = String.valueOf(Float.valueOf(x.toString()).intValue());
>+ break;
>+ }
>+ }
>+ return x;
>+ }
>+
>
>
>
>
>
>------------------------------------------------------------------------
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>
>

Dmitry Tkach wrote:

>
> s.setObject (1, "x'a'");
>
I meant s.setObject (1, "x'a'", Types.INTEGER) of course...

Dima

Can't you instead use setString(1, "x'a'")?

If not, this also brings up another thing. Did you want to treat "x'a'"
as a number now?

In any case, here is a revised version of the patch. =) Thanks for the
pointers.

Also, the remove radix thing is not meant for preventing SQL injection.
It is meant for this like:

create function integer_in(integer) ....

and then using things like select integer_in(1.11231E9)

Kim

On Fri, 2003-07-18 at 13:40, Dmitry Tkach wrote:
> Dmitry Tkach wrote:
>
> >
> > s.setObject (1, "x'a'");
> >
> I meant s.setObject (1, "x'a'", Types.INTEGER) of course...
>
> Dima
>
>

Kim Ho wrote:

>Can't you instead use setString(1, "x'a'")?
>
Nope - that will get converted into ... where id='x\'a\'' - that won't
be understood by the backend - it wants it *exactly* that way - x
outside the quotes, followed by a quoted hexademical number...

>
>If not, this also brings up another thing. Did you want to treat "x'a'"
>as a number now?
>
Yes, I did (and still do) :-)

>
>In any case, here is a revised version of the patch. =) Thanks for the
>pointers.
>
I must be missing something, but I don't see any difference with the
previous version ....

>
>Also, the remove radix thing is not meant for preventing SQL injection.
>It is meant for this like:
>
>create function integer_in(integer) ....
>
>and then using things like select integer_in(1.11231E9)
>
I understand that. I was just saying that adding quotes around the input
doesn't help much in preventing injections, but does take away valuable
functionality...

Dima

> >
> >In any case, here is a revised version of the patch. =) Thanks for the
> >pointers.
> >
> I must be missing something, but I don't see any difference with the
> previous version ....
>
There is an attempt to do a:
Double.valueOf(x.toString());

in bindNumbers to ensure that the object's string representation is a
number. This is what you were talking about before right?

About breaking functionality with hex.

I'm not sure about this, maybe we could get other opinions in
(specifically Dave and Barry). I'm not so sure that it should be
allowed. (I am not saying that it is not useful.)

Cheers,

Kim

I just can't understand why a call to setObject(1, someString, Types.NUMERIC) would scape the contents of my "someString" variable, as I specified that it's a number

On Fri, 18 Jul 2003 13:53:39 -0400
Dmitry Tkach <dmitry(at)openratings(dot)com> wrote:

> Kim Ho wrote:
>
> >Can't you instead use setString(1, "x'a'")?
> >
> Nope - that will get converted into ... where id='x\'a\'' - that won't
> be understood by the backend - it wants it *exactly* that way - x
> outside the quotes, followed by a quoted hexademical number...
>
> >
> >If not, this also brings up another thing. Did you want to treat "x'a'"
> >as a number now?
> >
> Yes, I did (and still do) :-)
>
> >
> >In any case, here is a revised version of the patch. =) Thanks for the
> >pointers.
> >
> I must be missing something, but I don't see any difference with the
> previous version ....
>
> >
> >Also, the remove radix thing is not meant for preventing SQL injection.
> >It is meant for this like:
> >
> >create function integer_in(integer) ....
> >
> >and then using things like select integer_in(1.11231E9)
> >
> I understand that. I was just saying that adding quotes around the input
> doesn't help much in preventing injections, but does take away valuable
> functionality...
>
> Dima
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faqs/FAQ.html

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

Kim Ho wrote:

>>>In any case, here is a revised version of the patch. =) Thanks for the
>>>pointers.
>>>
>>>
>>>
>>I must be missing something, but I don't see any difference with the
>>previous version ....
>>
>>
>>
>There is an attempt to do a:
>Double.valueOf(x.toString());
>
>in bindNumbers to ensure that the object's string representation is a
>number. This is what you were talking about before right?
>
>
Oh, I see...
Now you end up parsing it twice in some cases (once in removeRadix(),
and once again in bindNumber()) - why don't you merge those two together
to save one parse?

Not that I care, because I am not going to be able to use this at all
(and will have to build my own driver if it ever gets in), because it
breaks the existing functionality :-(

The hex thing may be questionable - although, I do sympathize with
people who may be using it currently, and will be forced to rewrite
their app now, but, at least, they have a (relatively) simple solution
(which is to take on the backend's parsing of hex numbers, and do it on
the app side)...
What I am concerned about is the "in" thing -

select * from sometable where x in ?;
setObject (1, "(1,2,3,4,5)");

that works just fine right now, and will be irreperably broken by this
patch...
No way I am going to rewrite all the existing code to do something like

select * from sometable where x in
(?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
... and then loop through the set and set each param separately, hoping
I have enough questionmarks to fit them all in :-)

Dima

>About breaking functionality with hex.
>
>I'm not sure about this, maybe we could get other opinions in
>(specifically Dave and Barry). I'm not so sure that it should be
>allowed. (I am not saying that it is not useful.)
>
>Cheers,
>
>Kim
>
>

Yes, If a patch prevents this use of setObject() for IN clauses, and also will HAVE to create my own driver, something I'm trying to avoid at all costs

On Fri, 18 Jul 2003 14:22:24 -0400
Dmitry Tkach <dmitry(at)openratings(dot)com> wrote:

> Kim Ho wrote:
>
> >>>In any case, here is a revised version of the patch. =) Thanks for the
> >>>pointers.
> >>>
> >>>
> >>>
> >>I must be missing something, but I don't see any difference with the
> >>previous version ....
> >>
> >>
> >>
> >There is an attempt to do a:
> >Double.valueOf(x.toString());
> >
> >in bindNumbers to ensure that the object's string representation is a
> >number. This is what you were talking about before right?
> >
> >
> Oh, I see...
> Now you end up parsing it twice in some cases (once in removeRadix(),
> and once again in bindNumber()) - why don't you merge those two together
> to save one parse?
>
> Not that I care, because I am not going to be able to use this at all
> (and will have to build my own driver if it ever gets in), because it
> breaks the existing functionality :-(
>
> The hex thing may be questionable - although, I do sympathize with
> people who may be using it currently, and will be forced to rewrite
> their app now, but, at least, they have a (relatively) simple solution
> (which is to take on the backend's parsing of hex numbers, and do it on
> the app side)...
> What I am concerned about is the "in" thing -
>
> select * from sometable where x in ?;
> setObject (1, "(1,2,3,4,5)");
>
> that works just fine right now, and will be irreperably broken by this
> patch...
> No way I am going to rewrite all the existing code to do something like
>
> select * from sometable where x in
> (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?)
> ... and then loop through the set and set each param separately, hoping
> I have enough questionmarks to fit them all in :-)
>
>
> Dima
>
> >About breaking functionality with hex.
> >
> >I'm not sure about this, maybe we could get other opinions in
> >(specifically Dave and Barry). I'm not so sure that it should be
> >allowed. (I am not saying that it is not useful.)
> >
> >Cheers,
> >
> >Kim
> >
> >
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 7: don't forget to increase your free space map settings

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

On Fri, Jul 18, 2003 at 02:22:24PM -0400, Dmitry Tkach wrote:

> What I am concerned about is the "in" thing -
>
> select * from sometable where x in ?;
> setObject (1, "(1,2,3,4,5)");
>
> that works just fine right now, and will be irreperably broken by this
> patch...

How about:

+ create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
+ use setObject(N, new InSet(myArray, Types.INTEGER));

The Javadoc for PreparedStatement.setObject() says:

Note that this method may be used to pass datatabase- specific abstract data
types, by using a driver-specific Java type.

so presumably this is the right way to do it. setObject() would handle an
InSet by individually escaping the components of the array it wraps as if
they had been passed to setObject() and turning them into an IN-like clause.

For arrays we just need to fix setArray() so that it does the same sort of
thing as described above, using the java.sql.Array methods to get the
component objects. And maybe provide a simple implementation of Array that
wraps a Java array (users can always provide their own, anyway).

Any thoughts? I can try to sort out patches, but I'm a bit short on time
right now so no promises.

-O

On Sat, Jul 19, 2003 at 03:03:20AM -0400, Aaron Mulder wrote:
> I have to agree that it would be better to provide a solution
> using API calls than a solutions that requires one to send SQL syntax
> into a ?.
>
> But why can't we kill both birds with an implementation of
> java.sql.Array? If you need to use an Array, you can use a PGArray that
> wraps an Object[]. If you want to do an in clause, do "where foo in (?)"
> or "where foo in ?" and then use ps.setArray() instead of setObject().
> Then we can make the docs explicit on this.

setArray() needs to generate "'{1,2,3}'" for an array field, but you want
"(1,2,3)" for an IN clause. Requiring the JDBC driver to parse the query to
distinguish the two cases seems awkward.

-O

I have to agree that it would be better to provide a solution
using API calls than a solutions that requires one to send SQL syntax
into a ?.

But why can't we kill both birds with an implementation of
java.sql.Array? If you need to use an Array, you can use a PGArray that
wraps an Object[]. If you want to do an in clause, do "where foo in (?)"
or "where foo in ?" and then use ps.setArray() instead of setObject().
Then we can make the docs explicit on this.

It does look like java.sql.Array is kind of overkill here, but we
can provide simple constructor that makes the common usage patterns easy,
and if you can't get a full ResultSet from your in clause in v1.0, so be
it.

Aaron

On Sat, 19 Jul 2003, Oliver Jowett wrote:
> How about:
>
> + create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
> + use setObject(N, new InSet(myArray, Types.INTEGER));
>
> The Javadoc for PreparedStatement.setObject() says:
>
> Note that this method may be used to pass datatabase- specific abstract data
> types, by using a driver-specific Java type.
>
> so presumably this is the right way to do it. setObject() would handle an
> InSet by individually escaping the components of the array it wraps as if
> they had been passed to setObject() and turning them into an IN-like clause.
>
> For arrays we just need to fix setArray() so that it does the same sort of
> thing as described above, using the java.sql.Array methods to get the
> component objects. And maybe provide a simple implementation of Array that
> wraps a Java array (users can always provide their own, anyway).
>
> Any thoughts? I can try to sort out patches, but I'm a bit short on time
> right now so no promises.
>
> -O
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)
>

To keep things simple, wouldn't it be reasonable to test the last SQL
fragment and see if it ends in " IN" (trimmed and case insensitively)
and use the parenthesis notation instead of the array one? I don't
think we can have a PostgreSQL array as the argument of an IN clause anyway.

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

The problem with this (and other similar suggestions in this thread -
like use PGArray etc.) is that the app will not even compile with
postgres jdbc classes.
The whole point in using jdbc interfaces is to abstract the application
from the particular driver implementation.

The way it works currently, it is entirely possible either (text
representation of arrays vary between vendors, as do ways you can use to
specify sets as parameters), but, at least, you don't have to maintain
the whole special piece of code, that can only compile when postgres
driver is available.

Dima

Oliver Jowett wrote:

>On Fri, Jul 18, 2003 at 02:22:24PM -0400, Dmitry Tkach wrote:
>
>
>
>>What I am concerned about is the "in" thing -
>>
>>select * from sometable where x in ?;
>>setObject (1, "(1,2,3,4,5)");
>>
>>that works just fine right now, and will be irreperably broken by this
>>patch...
>>
>>
>
>How about:
>
> + create an org.postgresql.InSet class that wraps an Object[] array and java.sql.Types value
> + use setObject(N, new InSet(myArray, Types.INTEGER));
>
>The Javadoc for PreparedStatement.setObject() says:
>
> Note that this method may be used to pass datatabase- specific abstract data
> types, by using a driver-specific Java type.
>
>so presumably this is the right way to do it. setObject() would handle an
>InSet by individually escaping the components of the array it wraps as if
>they had been passed to setObject() and turning them into an IN-like clause.
>
>For arrays we just need to fix setArray() so that it does the same sort of
>thing as described above, using the java.sql.Array methods to get the
>component objects. And maybe provide a simple implementation of Array that
>wraps a Java array (users can always provide their own, anyway).
>
>Any thoughts? I can try to sort out patches, but I'm a bit short on time
>right now so no promises.
>
>-O
>
>

On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
> The problem with this (and other similar suggestions in this thread -
> like use PGArray etc.) is that the app will not even compile with
> postgres jdbc classes.
> The whole point in using jdbc interfaces is to abstract the application
> from the particular driver implementation.

My current approach is what Fernando suggested -- use setArray() and look
for a preceeding IN. This can work without needing any postgres specific
classes -- I'll add a simple implementation of java.sql.Array that wraps a
Java array to the driver source, but if you don't want to be dependent on
the driver you can provide your own implementation.

The interface-abstraction argument only really works up to the point where
you want to do something not defined in the interface. Usually when I'm
doing DB-specific extension stuff I have a per-DB subclass that does the
special bits; if you don't have the driver available, you don't compile that
subclass. So I don't really buy the "can't build" argument. The same
thing applies to extensions like setUseServerPrepare(), BTW.

-O

Oliver Jowett wrote:

>On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>
>
>>The problem with this (and other similar suggestions in this thread -
>>like use PGArray etc.) is that the app will not even compile with
>>postgres jdbc classes.
>>The whole point in using jdbc interfaces is to abstract the application
>>from the particular driver implementation.
>>
>>
>
>My current approach is what Fernando suggested -- use setArray() and look
>for a preceeding IN. This can work without needing any postgres specific
>classes -- I'll add a simple implementation of java.sql.Array that wraps a
>Java array to the driver source, but if you don't want to be dependent on
>the driver you can provide your own implementation.
>
>
I don't want to provide my own iplementation either :-)
I was fairly happy with what it used to be - just call setObject () and
be done with it
Anything else does look like an overkill...

>The interface-abstraction argument only really works up to the point where
>you want to do something not defined in the interface. Usually when I'm
>doing DB-specific extension stuff I have a per-DB subclass that does the
>special bits; if you don't have the driver available, you don't compile that
>subclass. So I don't really buy the "can't build" argument. The same
>thing applies to extensions like setUseServerPrepare(), BTW.
>
Yep. That's exactly why I was arguing against having it to beging with
back in the beginning of 7.3
.. I lost that one too :-(
... but still believe it was the wrong idea.
I mean, as I said before, I do have do go an extra mile because of this
precompiled query plan stuff...
I am writing functions in C that cache query plans and return strings,
and call them from java, and pase strings back in rows... I just can't
afford casting my statements to org.jdbc.postgres.Something... sorry - I
can't write my apps assuming that postgres is the only db they'll ever
need :-(

Dima

(Cc: list trimmed)

On Sun, Jul 20, 2003 at 10:36:53PM -0400, Dima Tkach wrote:

> Yep. That's exactly why I was arguing against having it to beging with
> back in the beginning of 7.3
> .. I lost that one too :-(
> ... but still believe it was the wrong idea.
> I mean, as I said before, I do have do go an extra mile because of this
> precompiled query plan stuff...
> I am writing functions in C that cache query plans and return strings,
> and call them from java, and pase strings back in rows... I just can't
> afford casting my statements to org.jdbc.postgres.Something... sorry - I
> can't write my apps assuming that postgres is the only db they'll ever
> need :-(

Um, so you don't want to depend on the JDBC postgresql driver at all, but
don't mind having postgresql-specific behaviour elsewhere so long
as you can do it purely through the JDBC interface? That seems a bit
inconsistent.

Note that using setObject() to set IN clauses is no less postgresql-specific
than using setArray() would be .. I know of at least one DB where you really
can only set data values in a prepared statement as the prepared SQL is
compiled down to a form of bytecode for DB-side execution, there *is* no
textual substitution that goes on.

What I was suggesting was a scheme like:

interface JDBCExtensions {
void initStatement(PreparedStatement stmt);
void setInClause(PreparedStatement stmt, int index, int[] values);
}

class PostgresqlJDBCExceptions implements JDBCExtensions {
public void initStatement(PreparedStatement stmt) {
((org.postgresql.PGStatement)stmt).setUseServerPrepare(true);
}

public void setInClause(PreparedStatement stmt, int index, int[] values) {
stmt.setArray(index, new org.postgresql.jdbc2.WrappedArray(values, Types.INTEGER));
}
}

class FooJDBCExceptions implements JDBCExtensions {
// implementation for FooDB
}

// ...

Class datasourceClass = Class.forName(configuredDatasourceClass);
Class extensionsClass = Class.forName(configuredExtensionsClass);
JDBCExtensions extensions = extensionsClass.newInstance();

// ...

PreparedStatement stmt = connection.prepareStatement(...);
extensions.initStatement(stmt);
extensions.setInClause(stmt, 4, new int[] { 1,2,3,4 });

Then conditionalize compilation of implementations of JDBCExceptions on the
presence of the appropriate driver. When built, you get extension support
for all the drivers present on the build system. When running from a built
jar, only the configured extension class is loaded (and presumably the
end-user has the corresponding JDBC driver available, or they can't talk to
the DB anyway!)

What prevents you from taking this approach?

-O

On Mon, Jul 21, 2003 at 03:01:57PM +1200, Oliver Jowett wrote:

> class PostgresqlJDBCExceptions implements JDBCExtensions {
> class FooJDBCExceptions implements JDBCExtensions {
> Then conditionalize compilation of implementations of JDBCExceptions on the

s/Exceptions/Extensions/, of course :)

-O

Dima Tkach wrote:

> I was fairly happy with what it used to be - just call setObject () and
> be done with it

Unfortunately that is not an option as it is a security risk.

You cannot leave a driver out there which allows people to insert
potentially harmful SQL statements just to make it easier for someone to
specify a set.

In any case, I wonder if all PreparedStatements won't be server side
only one day as the client side interface was created to fill in for the
lack of that in older backends. Once that happens and the V3 protocol
is used (7.4+ backends) I doubt that SQL injection, and the hack to set
IN arguments, will work.

Regards to all,
Fernando

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

I think that the simplest thing would be to have an option in the
backend to disable processing of multiple statements in one query --
i.e. disallow the use of ';' as a separator of statements. I am not sure
why this feature (multiple statments in one query) is there anyway.
"Reduce network roundtrips" is the usual reply, but, then, what is the
purpose of stored procedures (functions in PostgreSQL)?

In my perception, JDBC is meant to be a client side extension of the
server - bridge for Java clients to use the server (which in our case is
the right and honorable PostgreSQL). Prepared statements is a mechanism
to indicate the server that more query of this kind is likely to come so
the plan of the query should be kept ready to be used again. That is
what meant by PreparedStatement in the JDBC driver. I find the concept
of "client side prepared statements" pretty weird.

From this perspective, the whole wrestling with "drop table..." and
similar risks seem farily vain to me. At least, the driver is not the
place to solve this kind of security problems which basically exist due
to the wya the server behaves. Instead, the question should be asked: is
the behaviour of the server optimal?. Do we need this feature
(processing multiple, semi-colon separated statements)? Should not this
feature be eventually optional?

Cheers,
Peter

Fernando Nasser wrote:

> Dima Tkach wrote:
>
>> I was fairly happy with what it used to be - just call setObject ()
>> and be done with it
>
>
> Unfortunately that is not an option as it is a security risk.
>
> You cannot leave a driver out there which allows people to insert
> potentially harmful SQL statements just to make it easier for someone
> to specify a set.
>
> In any case, I wonder if all PreparedStatements won't be server side
> only one day as the client side interface was created to fill in for
> the lack of that in older backends. Once that happens and the V3
> protocol is used (7.4+ backends) I doubt that SQL injection, and the
> hack to set IN arguments, will work.
>
> Regards to all,
> Fernando
>

Actually, here is what javadoc comments say in the jdk's
PreparedStatement class:

/**
* An object that represents a precompiled SQL statement.
* <P>A SQL statement is precompiled and stored in a
* <code>PreparedStatement</code> object. This object can then be used to
* efficiently execute this statement multiple times.
*
* <P><B>Note:</B> The setter methods (<code>setShort</code>,
<code>setString</code>,
* and so on) for setting IN parameter values
* must specify types that are compatible with the defined SQL type of
* the input parameter. For instance, if the IN parameter has SQL type
* <code>INTEGER</code>, then the method <code>setInt</code> should be used.
*
* <p>If arbitrary parameter type conversions are required, the method
* <code>setObject</code> should be used with a target SQL type.
* <P>
*/

Two things that stricke me here:

- no mention of "security" stuff whatsoever. The sole purpose of
PreparedStatement according to this is to "efficiently execute this
statement multipe times",
not "to prevent slq injection attacks" or anything like that;

- it is *explicitly* stated that setObject () should be used for
"arbitrary type conversions";

Dima

Peter Kovacs wrote:

> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not
> sure why this feature (multiple statments in one query) is there
> anyway. "Reduce network roundtrips" is the usual reply, but, then,
> what is the purpose of stored procedures (functions in PostgreSQL)?
>
> In my perception, JDBC is meant to be a client side extension of the
> server - bridge for Java clients to use the server (which in our case
> is the right and honorable PostgreSQL). Prepared statements is a
> mechanism to indicate the server that more query of this kind is
> likely to come so the plan of the query should be kept ready to be
> used again. That is what meant by PreparedStatement in the JDBC
> driver. I find the concept of "client side prepared statements" pretty
> weird.
>
> From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist
> due to the wya the server behaves. Instead, the question should be
> asked: is the behaviour of the server optimal?. Do we need this
> feature (processing multiple, semi-colon separated statements)? Should
> not this feature be eventually optional?
>
> Cheers,
> Peter
>
> Fernando Nasser wrote:
>
>> Dima Tkach wrote:
>>
>>> I was fairly happy with what it used to be - just call setObject ()
>>> and be done with it
>>
>>
>>
>> Unfortunately that is not an option as it is a security risk.
>>
>> You cannot leave a driver out there which allows people to insert
>> potentially harmful SQL statements just to make it easier for someone
>> to specify a set.
>>
>> In any case, I wonder if all PreparedStatements won't be server side
>> only one day as the client side interface was created to fill in for
>> the lack of that in older backends. Once that happens and the V3
>> protocol is used (7.4+ backends) I doubt that SQL injection, and the
>> hack to set IN arguments, will work.
>>
>> Regards to all,
>> Fernando
>>

Fernando Nasser wrote:

> Dima Tkach wrote:
>
>> I was fairly happy with what it used to be - just call setObject ()
>> and be done with it
>
>
> Unfortunately that is not an option as it is a security risk.
>
> You cannot leave a driver out there which allows people to insert
> potentially harmful SQL statements just to make it easier for someone
> to specify a set.

The driver allows people to "insert potentially harmful SQL" *anyway* -
even if every "problem" of this kind with PreparedStatement is fixed,
the *driver* still allows you to send in anything you want by simply
using Statement instead...

You can't possibly hope that JDBC driver will take care of alll of the
security risks for you. If you don't know how to write safe code, you'll
be doomed. If you do, then you do not need help from jdbc driver. JDBC
driver's whole purpose is to provide an abstraction layer between a
database and an application program.
It has nothing to do with security whatsoever.

Dima

>
>
>Um, so you don't want to depend on the JDBC postgresql driver at all, but
>don't mind having postgresql-specific behaviour elsewhere so long
>as you can do it purely through the JDBC interface? That seems a bit
>inconsistent.
>
>
I don't see anything "inconsistent" about it.
I am just choosing the lesser of two evils.
At least, I don't need to know what database I am going to be working
with when I compile the code.

>Note that using setObject() to set IN clauses is no less postgresql-specific
>than using setArray() would be .. I know of at least one DB where you really
>can only set data values in a prepared statement as the prepared SQL is
>compiled down to a form of bytecode for DB-side execution, there *is* no
>textual substitution that goes on.
>
Right. I do need a scheme like what you suggest below to set up
database-specific logic.
The difference is - I do not need all the possible jdbc drivers to be
installed just to compile my application.
And I do not need to know which driver a particular customer is using to
send him my application.
And I don't need to force my customers to have to request a new app from
me if they decide to switch to another database.

With your suggestion, the only way I can have the above is to pack all
the possible jdbc drivers into every build of the app...

Dima

>
>What I was suggesting was a scheme like:
>
> interface JDBCExtensions {
> void initStatement(PreparedStatement stmt);
> void setInClause(PreparedStatement stmt, int index, int[] values);
> }
>
> class PostgresqlJDBCExceptions implements JDBCExtensions {
> public void initStatement(PreparedStatement stmt) {
> ((org.postgresql.PGStatement)stmt).setUseServerPrepare(true);
> }
>
> public void setInClause(PreparedStatement stmt, int index, int[] values) {
> stmt.setArray(index, new org.postgresql.jdbc2.WrappedArray(values, Types.INTEGER));
> }
> }
>
> class FooJDBCExceptions implements JDBCExtensions {
> // implementation for FooDB
> }
>
> // ...
>
> Class datasourceClass = Class.forName(configuredDatasourceClass);
> Class extensionsClass = Class.forName(configuredExtensionsClass);
> JDBCExtensions extensions = extensionsClass.newInstance();
>
> // ...
>
> PreparedStatement stmt = connection.prepareStatement(...);
> extensions.initStatement(stmt);
> extensions.setInClause(stmt, 4, new int[] { 1,2,3,4 });
>
>Then conditionalize compilation of implementations of JDBCExceptions on the
>presence of the appropriate driver. When built, you get extension support
>for all the drivers present on the build system. When running from a built
>jar, only the configured extension class is loaded (and presumably the
>end-user has the corresponding JDBC driver available, or they can't talk to
>the DB anyway!)
>
>What prevents you from taking this approach?
>
>-O
>
>

Oliver Jowett wrote:

>On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>
>
>>The problem with this (and other similar suggestions in this thread -
>>like use PGArray etc.) is that the app will not even compile with
>>postgres jdbc classes.
>>The whole point in using jdbc interfaces is to abstract the application
>>from the particular driver implementation.
>>
>>
>
>My current approach is what Fernando suggested -- use setArray() and look
>for a preceeding IN. This can work without needing any postgres specific
>classes -- I'll add a simple implementation of java.sql.Array that wraps a
>Java array to the driver source, but if you don't want to be dependent on
>the driver you can provide your own implementation.
>
>
Why not just allow setObject() to take Collection as an argument?
You would not need any special implementations then... and the
application would not need to waste cycles on wrapping/unwrapping those
Arrays every time...

Dima

Dmitry Tkach wrote:
>
> Two things that stricke me here:
>
> - no mention of "security" stuff whatsoever. The sole purpose of
> PreparedStatement according to this is to "efficiently execute this
> statement multipe times",
> not "to prevent slq injection attacks" or anything like that;
>

Because in "real" prepared statements there is no such risk. The risk is the
artifact of a bug in our client side simulation of prepared statements (not real
prepared statements as per definition).

> - it is *explicitly* stated that setObject () should be used for
> "arbitrary type conversions";
>

Not that arbitrary. There is a table specifying for each java type that the
passed object is member of the proper JDBC type for the converted result. Which
must be the type of the field you are trying to specify the value for.

So it is not that arbitrary.

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:

> You can't possibly hope that JDBC driver will take care of alll of the
> security risks for you. If you don't know how to write safe code, you'll
> be doomed. If you do, then you do not need help from jdbc driver. JDBC
> driver's whole purpose is to provide an abstraction layer between a
> database and an application program.
> It has nothing to do with security whatsoever.

This is only true if all DBs use identical SQL syntax, which they don't.
Tried embedding a NUL into a query lately?

Even if it was true, it's still better to have one piece of code that does
the escaping, rather than N different ones. With escaping in the JDBC
driver, you've reduced the scope of the code you need to audit for syntax
from "all query strings and all parameters" to "the JDBC driver's
parameter-escaping code and all query strings".

-O

Fernando Nasser wrote:

> Dmitry Tkach wrote:
>
>>
>> Two things that stricke me here:
>>
>> - no mention of "security" stuff whatsoever. The sole purpose of
>> PreparedStatement according to this is to "efficiently execute this
>> statement multipe times",
>> not "to prevent slq injection attacks" or anything like that;
>>
>
> Because in "real" prepared statements there is no such risk. The risk
> is the artifact of a bug in our client side simulation of prepared
> statements (not real prepared statements as per definition).

My point was that the risk exists, when you do *not* use
PreparedStatements, right?
If the purpose of PreparedStatement was to eliminate that risk, it would
have been mentioned. But it is not. Because PreparedStatement has
nothing to do with the security. It is all about efficiency.

>> - it is *explicitly* stated that setObject () should be used for
>> "arbitrary type conversions";
>>
>
> Not that arbitrary. There is a table specifying for each java type
> that the passed object is member of the proper JDBC type for the
> converted result. Which must be the type of the field you are trying
> to specify the value for.
>
> So it is not that arbitrary.

It doesn't say *how* arbitrary. It just says "arbitrary". :-)
If you could only pass objects of types in that table, you would not
need setObject () - just setString(), setInt() etc... would suffice.
The whole idea of setObject () is to be able to pass in an argument for
each there is no specialized setter function.

Dima

Peter Kovacs wrote:> I think that the simplest thing would be to have an option
in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not sure
> why this feature (multiple statments in one query) is there anyway.
> "Reduce network roundtrips" is the usual reply, but, then, what is the
> purpose of stored procedures (functions in PostgreSQL)?
>

I don't think the backend maintainers will like that. And btw we don't need
this at all.

1) There is no risk of SQL injection with "real" (or "server side") prepared
statements.

2) With the proper implementation of the client side prepared statements
emulation the injected statements will not go through, because:

a) all non-string results are properly quoted and generated from primary types.

b) String type contents are quoted and have its quotes escaped.

> In my perception, JDBC is meant to be a client side extension of the
> server - bridge for Java clients to use the server (which in our case is
> the right and honorable PostgreSQL). Prepared statements is a mechanism
> to indicate the server that more query of this kind is likely to come so
> the plan of the query should be kept ready to be used again. That is
> what meant by PreparedStatement in the JDBC driver. I find the concept
> of "client side prepared statements" pretty weird.
>

It was added before the server had prepared statements so the applications could
still use PreparedStatements and the getXXX and setXXX methods instead of
recreating strings with queries all the time. Some of the applications run with
other databases as well.

> From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist due
> to the wya the server behaves. Instead, the question should be asked: is
> the behaviour of the server optimal?. Do we need this feature
> (processing multiple, semi-colon separated statements)? Should not this
> feature be eventually optional?
>

No SQL injection is possible with the backend implementation of prepared
statements (with the V3 protocol, at least).

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:

> Why not just allow setObject() to take Collection as an argument?

You need information on the SQL type of the contents to be able to turn them
into a DB representation correctly. We can't use the type parameter to
setObject() for this as that should reflect the whole paramater, i.e.
probably Types.OTHER in this case.

java.sql.Array has a getBaseType() that does the job.

> You would not need any special implementations then... and the
> application would not need to waste cycles on wrapping/unwrapping those
> Arrays every time...

Hah, and it's faster to wrap your array of ints in a bunch of
java.lang.Integer objects so you can put in in a Collection? :)

-O

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:
>
>
>
>>You can't possibly hope that JDBC driver will take care of alll of the
>>security risks for you. If you don't know how to write safe code, you'll
>>be doomed. If you do, then you do not need help from jdbc driver. JDBC
>>driver's whole purpose is to provide an abstraction layer between a
>>database and an application program.
>>It has nothing to do with security whatsoever.
>>
>>
>
>This is only true if all DBs use identical SQL syntax, which they don't.
>Tried embedding a NUL into a query lately?
>
If you use standard SQL, and standard compliant database, you should be ok.
If you use certain db-specific extensions, you'll still benefit from
JDBC, abstracting *most* of your sql for you.

My point was that it has nothing to do with security anyway. :-)
I was not planning to start discussing how much abstraction it provides.
I agree, that it could be better.

>Even if it was true, it's still better to have one piece of code that does
>the escaping, rather than N different ones. With escaping in the JDBC
>driver, you've reduced the scope of the code you need to audit for syntax
>from "all query strings and all parameters" to "the JDBC driver's
>parameter-escaping code and all query strings".
>
>
>

Sure. And that's good.
That's precisely the point - if you guys start taking functionality
away, so that I am not longer able to do things with it that I used to
be able to do, then I will not be able to benefit from it as much as I
used to - I'll have to switch from PreparedStatements to Statements and
do all that escaping/parsing on my own.
That's exactly what I am trying to avoid

Dima

Dmitry Tkach wrote:> Oliver Jowett wrote:
>
>> On Sun, Jul 20, 2003 at 12:39:45PM -0400, Dima Tkach wrote:
>>
>>
>>> The problem with this (and other similar suggestions in this thread -
>>> like use PGArray etc.) is that the app will not even compile with
>>> postgres jdbc classes.
>>> The whole point in using jdbc interfaces is to abstract the
>>> application from the particular driver implementation.
>>>
>>
>>
>> My current approach is what Fernando suggested -- use setArray() and look
>> for a preceeding IN. This can work without needing any postgres specific
>> classes -- I'll add a simple implementation of java.sql.Array that
>> wraps a
>> Java array to the driver source, but if you don't want to be dependent on
>> the driver you can provide your own implementation.
>>
>>
> Why not just allow setObject() to take Collection as an argument?
> You would not need any special implementations then... and the
> application would not need to waste cycles on wrapping/unwrapping those
> Arrays every time...
>

That is an interesting idea. But how do we know the type of the elements that
should go in the list? We will just get java.Objects as we go through the
Collection.

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
>
>
>
>>Why not just allow setObject() to take Collection as an argument?
>>
>>
>
>You need information on the SQL type of the contents to be able to turn them
>into a DB representation correctly. We can't use the type parameter to
>setObject() for this as that should reflect the whole paramater, i.e.
>probably Types.OTHER in this case.
>
It doesn't seem to be required anywhere - it just says "the type to be
sent to the database" in the description of that
argument. You can interpret it to be the type of the contents when
dealing with collections/sets/arrays

>
>Hah, and it's faster to wrap your array of ints in a bunch of
>java.lang.Integer objects so you can put in in a Collection? :)
>
>
>
Maybe, I had a List of Integers to begin with?
Or, perhaps, you'll be so kind to provide a smart implementation that
will understand arrays too, not just collections :-)

Dima

On Mon, Jul 21, 2003 at 10:24:15AM -0400, Dmitry Tkach wrote:

> >Note that using setObject() to set IN clauses is no less
> >postgresql-specific
> >than using setArray() would be .. I know of at least one DB where you
> >really
> >can only set data values in a prepared statement as the prepared SQL is
> >compiled down to a form of bytecode for DB-side execution, there *is* no
> >textual substitution that goes on.
> >
> Right. I do need a scheme like what you suggest below to set up
> database-specific logic.
> The difference is - I do not need all the possible jdbc drivers to be
> installed just to compile my application.
> And I do not need to know which driver a particular customer is using to
> send him my application.
> And I don't need to force my customers to have to request a new app from
> me if they decide to switch to another database.
>
> With your suggestion, the only way I can have the above is to pack all
> the possible jdbc drivers into every build of the app...

Pack the support code for all drivers in, yes. The drivers themselves you
don't need to provide, as the support classes don't get loaded unless the
particular driver it supports is present.

There's no reason you can't have a "generic" extension implementation that
works on a plain JDBC driver -- you just don't get the benefit of the
extensions. Default to that for drivers you don't recognize at runtime.

If your app doesn't work without a real implementation of the extensions,
then you're not going to be able to just plug in a new JDBC driver and have
it work *regardless of how you get at those extensions* -- you have an
application that requires more JDBC provides, end of story. If you're in
this situation but want to allow your customers to run on arbitary DBs
without your intervention -- document your Extensions interface and release
it to your customers; they can build their own implementation for whatever
DB they like.

-O

>
>
>>>
>> Why not just allow setObject() to take Collection as an argument?
>> You would not need any special implementations then... and the
>> application would not need to waste cycles on wrapping/unwrapping
>> those Arrays every time...
>>
>
> That is an interesting idea. But how do we know the type of the
> elements that should go in the list? We will just get java.Objects as
> we go through the Collection.
>
>
You can require the 'type' argument to setObject() to specify the target
type.

Besides, knowing the exact type doesn't really matter much, because you
aare going to be quoting everything anyways :-)

Dima

That's, then, even simpler that I originally thought. The only thing to
be done is to make using "real" prepared statements the default
behaviour of the PreparedStatement instances, is not it?

Fernando Nasser wrote:

> Peter Kovacs wrote:> I think that the simplest thing would be to have
> an option in the
>
>> backend to disable processing of multiple statements in one query --
>> i.e. disallow the use of ';' as a separator of statements. I am not
>> sure why this feature (multiple statments in one query) is there
>> anyway. "Reduce network roundtrips" is the usual reply, but, then,
>> what is the purpose of stored procedures (functions in PostgreSQL)?
>>
>
> I don't think the backend maintainers will like that. And btw we
> don't need this at all.
>
> 1) There is no risk of SQL injection with "real" (or "server side")
> prepared statements.
>
> 2) With the proper implementation of the client side prepared
> statements emulation the injected statements will not go through,
> because:
>
> a) all non-string results are properly quoted and generated from
> primary types.
>
> b) String type contents are quoted and have its quotes escaped.
>
>> In my perception, JDBC is meant to be a client side extension of the
>> server - bridge for Java clients to use the server (which in our case
>> is the right and honorable PostgreSQL). Prepared statements is a
>> mechanism to indicate the server that more query of this kind is
>> likely to come so the plan of the query should be kept ready to be
>> used again. That is what meant by PreparedStatement in the JDBC
>> driver. I find the concept of "client side prepared statements"
>> pretty weird.
>>
>
> It was added before the server had prepared statements so the
> applications could still use PreparedStatements and the getXXX and
> setXXX methods instead of recreating strings with queries all the
> time. Some of the applications run with other databases as well.
>
>> From this perspective, the whole wrestling with "drop table..." and
>> similar risks seem farily vain to me. At least, the driver is not the
>> place to solve this kind of security problems which basically exist
>> due to the wya the server behaves. Instead, the question should be
>> asked: is the behaviour of the server optimal?. Do we need this
>> feature (processing multiple, semi-colon separated statements)?
>> Should not this feature be eventually optional?
>>
>
> No SQL injection is possible with the backend implementation of
> prepared statements (with the V3 protocol, at least).
>
>
>

>
>
>Pack the support code for all drivers in, yes. The drivers themselves you
>don't need to provide, as the support classes don't get loaded unless the
>particular driver it supports is present.
>
>
I need to *compile* all of that code.
And to *compile* it, I'll need to have all of those drivers available...
and I'll need all the possible versions of those drivers too, because I
don't know which version the customer will be using...

This is just simply impossible to do.

Dima

See my comments below.

On Mon, 2003-07-21 at 16:24, Dmitry Tkach wrote:
> >
> >
> >Um, so you don't want to depend on the JDBC postgresql driver at all, but
> >don't mind having postgresql-specific behaviour elsewhere so long
> >as you can do it purely through the JDBC interface? That seems a bit
> >inconsistent.
> >
> >
> I don't see anything "inconsistent" about it.

But please... did you ever use a different database than Postgres ? The
way you're passing the parameters is VERY much postgres specific,
furthermore, it relies on some non-standard behavior...

> I am just choosing the lesser of two evils.
> At least, I don't need to know what database I am going to be working
> with when I compile the code.

... which problem you wouldn't have if you would use standard JDBC code.
I agree that standards suck sometime, but then if you want to use a DB
specific feature, I can't see why not make it a compile time necessity -
it might be a further benefit to remind you that your code will not work
with other databases.

>
> >Note that using setObject() to set IN clauses is no less postgresql-specific
> >than using setArray() would be .. I know of at least one DB where you really
> >can only set data values in a prepared statement as the prepared SQL is
> >compiled down to a form of bytecode for DB-side execution, there *is* no
> >textual substitution that goes on.
> >
> Right. I do need a scheme like what you suggest below to set up
> database-specific logic.
> The difference is - I do not need all the possible jdbc drivers to be
> installed just to compile my application.
> And I do not need to know which driver a particular customer is using to
> send him my application.
> And I don't need to force my customers to have to request a new app from
> me if they decide to switch to another database.

But yes you force your customers to use Postgres, or maybe to a set of
databases which accept this hack - it is not standard.

>
> With your suggestion, the only way I can have the above is to pack all
> the possible jdbc drivers into every build of the app...

Yes, all the drivers which you are using special features from. That
makes perfect sense - those are vendor specific dependencies in your
code, the library will remind you about it all the time so you don't
forget to tell the customers about it ;-)

Cheers,
Csaba.

On Mon, Jul 21, 2003 at 10:49:19AM -0400, Dmitry Tkach wrote:
> >
> >
> >Pack the support code for all drivers in, yes. The drivers themselves you
> >don't need to provide, as the support classes don't get loaded unless the
> >particular driver it supports is present.
> >
> >
> I need to *compile* all of that code.
> And to *compile* it, I'll need to have all of those drivers available...
> and I'll need all the possible versions of those drivers too, because I
> don't know which version the customer will be using...
>
> This is just simply impossible to do.

So you have to pick a set of drivers to support and go with that. If you have
multiple interface versions that are mutually incompatible, build multiple
versions of your extensions independantly of each other. Yes, it's messy to
build, but not impossible and certainly no worse than any other situation
where you have to support multiple versions of a dependent library.

If you don't want to support a large number of driver versions, those
versions you don't support can be handled by the "generic" implementation I
suggested in my previous email.

I'm not sure this argument is really relevant to "what should setObject()
vs. setArray() do?" any more. Replies off-list, please.

-O

On Tue, 22 Jul 2003 02:30:02 +1200 Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
> On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:

> > You can't possibly hope that JDBC driver will take care of alll of the
> > security risks for you. If you don't know how to write safe code,
> you'll
> > be doomed. If you do, then you do not need help from jdbc driver. JDBC
> > driver's whole purpose is to provide an abstraction layer between a
> > database and an application program.
> > It has nothing to do with security whatsoever.
...
> Even if it was true, it's still better to have one piece of code that
> does
> the escaping, rather than N different ones. With escaping in the JDBC
> driver, you've reduced the scope of the code you need to audit for syntax
> from "all query strings and all parameters" to "the JDBC driver's
> parameter-escaping code and all query strings".

eewwww.

in a multi-tier architecture where the code that actually talks to
the database is isolated from the GUI, this is a totally unreasonable
expectation -- you really need to audit fields in the GUI, not somewhere
way back in the code.

even if PostgreSQL's jdbc driver somehow had wonderful code to handle
security problems, sensible DB independent code will _still_ need to audit
in the GUI because there is no reasonable expectation that all jdbc drivers
that might be used will have similar code.

i understand your desire for a single point of control, but moving this
into the jdbc driver is simply wrong. there are simply better ways;
java/swing/javabeans are powerful tools.

richard
--
Richard Welty rwelty(at)averillpark(dot)net
Averill Park Networking 518-573-7592
Java, PHP, PostgreSQL, Unix, Linux, IP Network Engineering, Security

Dmitry Tkach wrote:> Fernando Nasser wrote:
>
>> Dmitry Tkach wrote:
>>
>>>
>>> Two things that stricke me here:
>>>
>>> - no mention of "security" stuff whatsoever. The sole purpose of
>>> PreparedStatement according to this is to "efficiently execute this
>>> statement multipe times",
>>> not "to prevent slq injection attacks" or anything like that;
>>>
>>
>> Because in "real" prepared statements there is no such risk. The risk
>> is the artifact of a bug in our client side simulation of prepared
>> statements (not real prepared statements as per definition).
>
>
> My point was that the risk exists, when you do *not* use
> PreparedStatements, right?
> If the purpose of PreparedStatement was to eliminate that risk, it would
> have been mentioned. But it is not. Because PreparedStatement has
> nothing to do with the security. It is all about efficiency.
>

I don't agree with your reading. It is not mentioned because it is
intrinsically safe.

>
>>> - it is *explicitly* stated that setObject () should be used for
>>> "arbitrary type conversions";
>>>
>>
>> Not that arbitrary. There is a table specifying for each java type
>> that the passed object is member of the proper JDBC type for the
>> converted result. Which must be the type of the field you are trying
>> to specify the value for.
>>
>> So it is not that arbitrary.
>
>
> It doesn't say *how* arbitrary. It just says "arbitrary". :-)
> If you could only pass objects of types in that table, you would not
> need setObject () - just setString(), setInt() etc... would suffice.
> The whole idea of setObject () is to be able to pass in an argument for
> each there is no specialized setter function.
>

No, you are misreading the spec. The catch all is there, java class, which
result in JAVA_OBJECT.

The setObject method is intended to allow conversion between types, which is not
possible with the type specific setXXX that always convert to the default type
for that method.

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

On Mon, Jul 21, 2003 at 10:43:28AM -0400, Dmitry Tkach wrote:
> Oliver Jowett wrote:
>
> >On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
> >
> >
> >
> >>Why not just allow setObject() to take Collection as an argument?
> >>
> >>
> >
> >You need information on the SQL type of the contents to be able to turn
> >them
> >into a DB representation correctly. We can't use the type parameter to
> >setObject() for this as that should reflect the whole paramater, i.e.
> >probably Types.OTHER in this case.
> >
> It doesn't seem to be required anywhere - it just says "the type to be
> sent to the database" in the description of that
> argument. You can interpret it to be the type of the contents when
> dealing with collections/sets/arrays

java.sql.Types has pretty explicit mappings between type values and SQL
types. A collection of integers is definitely not a SQL INTEGER.

Types.OTHER says:

The constant in the Java programming language that indicates that the SQL
type is database-specific and gets mapped to a Java object that can be
accessed via the methods getObject and setObject.

which fits this use of Collection.

Responding to comments elsewhere .. you do need the component type to behave
correctly. For example, these produce different results:

setObject(1, new Date(...), Types.STRING)
setObject(1, new Date(...), Types.TIMESTAMP)

-O

On Mon, Jul 21, 2003 at 10:39:11AM -0400, Dmitry Tkach wrote:
> Oliver Jowett wrote:
>
> >Even if it was true, it's still better to have one piece of code that does
> >the escaping, rather than N different ones. With escaping in the JDBC
> >driver, you've reduced the scope of the code you need to audit for syntax
> >from "all query strings and all parameters" to "the JDBC driver's
> >parameter-escaping code and all query strings".
> >
> >
> >
>
> Sure. And that's good.
> That's precisely the point - if you guys start taking functionality
> away, so that I am not longer able to do things with it that I used to
> be able to do, then I will not be able to benefit from it as much as I
> used to - I'll have to switch from PreparedStatements to Statements and
> do all that escaping/parsing on my own.
> That's exactly what I am trying to avoid

The functionality we are "taking away" allows you to bypass the JDBC
driver's parameter escaping. You can't have it both ways.

-O

> Sure. And that's good.
> That's precisely the point - if you guys start taking functionality
> away, so that I am not longer able to do things with it that I used to
> be able to do, then I will not be able to benefit from it as much as I
> used to - I'll have to switch from PreparedStatements to Statements and
> do all that escaping/parsing on my own.
> That's exactly what I am trying to avoid

Dima, you are talking about "functionality" which is not documented,
discovered by yourself as working. This is similar with directly using
the com.sun.* classes - might work now, but will be a major PITA when
sun decides to change their internal API. You should have expected this
outcome... and it's certainly not an argument against fixing the driver
to be standards compliant...

Cheers,
Csaba.

>
> Dima
>
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly
>

On Mon, Jul 21, 2003 at 11:01:56AM -0400, Richard Welty wrote:
> On Tue, 22 Jul 2003 02:30:02 +1200 Oliver Jowett <oliver(at)opencloud(dot)com> wrote:
> > On Mon, Jul 21, 2003 at 10:18:19AM -0400, Dmitry Tkach wrote:
>
> > > You can't possibly hope that JDBC driver will take care of alll of the
> > > security risks for you. If you don't know how to write safe code,
> > you'll
> > > be doomed. If you do, then you do not need help from jdbc driver. JDBC
> > > driver's whole purpose is to provide an abstraction layer between a
> > > database and an application program.
> > > It has nothing to do with security whatsoever.
> ...
> > Even if it was true, it's still better to have one piece of code that
> > does
> > the escaping, rather than N different ones. With escaping in the JDBC
> > driver, you've reduced the scope of the code you need to audit for syntax
> > from "all query strings and all parameters" to "the JDBC driver's
> > parameter-escaping code and all query strings".
>
> eewwww.
>
> in a multi-tier architecture where the code that actually talks to
> the database is isolated from the GUI, this is a totally unreasonable
> expectation -- you really need to audit fields in the GUI, not somewhere
> way back in the code.

I was very careful to say "audit for syntax". You certainly want to make
sure you have input validation earlier on, too! -- but you don't need to
worry about, for example, correctly escaping strings that could validly have
a bare "'" in them before you pass them to the DB.

-O

>
>
>But please... did you ever use a different database than Postgres ? The
>way you're passing the parameters is VERY much postgres specific,
>furthermore, it relies on some non-standard behavior...
>
>
Yes. That's what I meant below - 'the lesser of two evils'

>
>
>>I am just choosing the lesser of two evils.
>>At least, I don't need to know what database I am going to be working
>>with when I compile the code.
>>
>>
>
>... which problem you wouldn't have if you would use standard JDBC code.
>
True. I would not have this problem then. I would have another problem -
just no way to pass a set or array in whatsoever.... :-)
I know a better way to not have problems - if I just didn't do anything
at all, I would not have any. :-)

>I agree that standards suck sometime, but then if you want to use a DB
>specific feature, I can't see why not make it a compile time necessity -
>it might be a further benefit to remind you that your code will not work
>with other databases.
>
I explained the reason for not making it a 'compile time necessity'
earlier - there is no way whatsoever I can have all possible jdbc
drivers in the world available to me at compile time.

>But yes you force your customers to use Postgres, or maybe to a set of
>databases which accept this hack - it is not standard.
>
>
I don't force them to use Postgres.
I do have the driver-specific logic that uses the correct way to pass in
the parameters depending on the driver.
The difference is that this is *run-time* logic, not *compilation-time*.

>
>
>>With your suggestion, the only way I can have the above is to pack all
>>the possible jdbc drivers into every build of the app...
>>
>>
>
>Yes, all the drivers which you are using special features from. That
>makes perfect sense - those are vendor specific dependencies in your
>code, the library will remind you about it all the time so you don't
>forget to tell the customers about it ;-)
>
>
It doesn't matter really if it makes sense or not (to me, it doesn't
anyway).
It is just not possible to do. I simply do not have all the drivers in
the world available to me.
Even if I did, I can't begin to imagine what my customers would tell
when they finish laughing after receiving an app from me, with all the
possible drivers built in :-)

As for not forgetting to tell the customers about it - that's the whole
idea - I am *not* telling the customers about it, they don't want to
know that, and don't need to care.

Dima

Peter Kovacs wrote:> That's, then, even simpler that I originally thought. The
only thing to
> be done is to make using "real" prepared statements the default
> behaviour of the PreparedStatement instances, is not it?
>

I agree with you. I would make it the default to 7.4 backends at least...
But it is up to Barry and Dave to decide.

We will probably have to keep the client side emulation around for a while.
Even if all the supported backend versions already have server side prepared
statements that implementation is recent enogh for us to have an alternative one
around, even if for comparison purposes. Besides, someone was claiming to have,
in some situations, faster results with the client side emulation. I believe
the 7.4 backend and the V3 protocol will solve that but we must be sure that is
so before removing this code (7.6 version will only have to support 7.3 and 7.4
backends so it is a possible end of the line for it).

As we have to keep the old code around, even if not the default, we will have to
fix it so nobody gets hurt.

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Oliver Jowett wrote:

>On Mon, Jul 21, 2003 at 10:39:11AM -0400, Dmitry Tkach wrote:
>
>
>>Oliver Jowett wrote:
>>
>>
>>
>>>Even if it was true, it's still better to have one piece of code that does
>>>the escaping, rather than N different ones. With escaping in the JDBC
>>>driver, you've reduced the scope of the code you need to audit for syntax
>>>
>>>
>>>from "all query strings and all parameters" to "the JDBC driver's
>>
>>
>>>parameter-escaping code and all query strings".
>>>
>>>
>>>
>>>
>>>
>>Sure. And that's good.
>>That's precisely the point - if you guys start taking functionality
>>away, so that I am not longer able to do things with it that I used to
>>be able to do, then I will not be able to benefit from it as much as I
>>used to - I'll have to switch from PreparedStatements to Statements and
>>do all that escaping/parsing on my own.
>>That's exactly what I am trying to avoid
>>
>>
>
>The functionality we are "taking away" allows you to bypass the JDBC
>driver's parameter escaping. You can't have it both ways.
>
>
Sure, I can :-)
I *do* "have it both ways" right now :-) - in situations when I need
drivers escaping, I use it, in situations when I don't I don't.
I have both the functionality, and the flexibility not to use it when I
don't need it.

Dima

>
>

> eewwww.
>
> in a multi-tier architecture where the code that actually talks to
> the database is isolated from the GUI, this is a totally unreasonable
> expectation -- you really need to audit fields in the GUI, not somewhere
> way back in the code.

Which it should be done indeed, but you also can't expect that a
middle-ware can correctly escape an input string against injection
attacks, as it can't know all the JDBC drivers it will talk to... this
is the job of the JDBC driver, the app talking to it should not even
attempt this.

>
> even if PostgreSQL's jdbc driver somehow had wonderful code to handle
> security problems, sensible DB independent code will _still_ need to audit
> in the GUI because there is no reasonable expectation that all jdbc drivers
> that might be used will have similar code.
>
This is not just about security problems fixed, it's about deterministic
behavior. If you have a non standard driver, you will not know how it
behaves unless you try out every possible input, and even less how it
will behave tomorrow. That's why is so important to have standards
compliance.

> i understand your desire for a single point of control, but moving this
> into the jdbc driver is simply wrong. there are simply better ways;
> java/swing/javabeans are powerful tools.
>

Yes, the application has to validate it's data, but this has nothing to
do with the JDBC validation. There could be perfectly valid data from
the application point of view which can result in unexpected results if
the driver doesn't do it's validation job correctly.

Cheers,
Csaba.

> richard
> --
> Richard Welty rwelty(at)averillpark(dot)net
> Averill Park Networking 518-573-7592
> Java, PHP, PostgreSQL, Unix, Linux, IP Network Engineering, Security
>
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>

>
>
>Dima, you are talking about "functionality" which is not documented,
>discovered by yourself as working. This is similar with directly using
>the com.sun.* classes - might work now, but will be a major PITA when
>sun decides to change their internal API.
>
I know. I understood the risk when I took that approach. And I am not
complaining.
If you had good reasons to take that functionality away, I would not
mind at all.
In this case though, it just looks like you are going to take it out
"just because", and I think, it is a bad idea. That's all.

>You should have expected this
>outcome... and it's certainly not an argument against fixing the driver
>to be standards compliant...
>
>
>
What standard are you talking about?
Where does it say anything about the driver being *required* to provide
no way for people to specify a set as a parameter?
Or where does it even mention anything about the requirement to quote
every single parameter the user sends in?

If you can show me a standard, that *requires* either of the above (not
just is silent about it), I'll have to give up, and agree that I am wrong.

Dima

Oliver Jowett wrote:> On Mon, Jul 21, 2003 at 10:43:28AM -0400, Dmitry Tkach wrote:
>
>>Oliver Jowett wrote:
>>
>>
>>>On Mon, Jul 21, 2003 at 10:27:30AM -0400, Dmitry Tkach wrote:
>>>
>>>
>>>
>>>
>>>>Why not just allow setObject() to take Collection as an argument?
>>>>
>>>>
>>>
>>>You need information on the SQL type of the contents to be able to turn
>>>them
>>>into a DB representation correctly. We can't use the type parameter to
>>>setObject() for this as that should reflect the whole paramater, i.e.
>>>probably Types.OTHER in this case.
>>>
>>
>>It doesn't seem to be required anywhere - it just says "the type to be
>>sent to the database" in the description of that
>>argument. You can interpret it to be the type of the contents when
>>dealing with collections/sets/arrays
>
>
> java.sql.Types has pretty explicit mappings between type values and SQL
> types. A collection of integers is definitely not a SQL INTEGER.
>
> Types.OTHER says:
>
> The constant in the Java programming language that indicates that the SQL
> type is database-specific and gets mapped to a Java object that can be
> accessed via the methods getObject and setObject.
>
> which fits this use of Collection.
>
> Responding to comments elsewhere .. you do need the component type to behave
> correctly. For example, these produce different results:
>
> setObject(1, new Date(...), Types.STRING)
> setObject(1, new Date(...), Types.TIMESTAMP)
>

Oliver,

I think Dima is arguing that Collection could be treated as an special case
where it indicates a list of something instead of a single something.

So, we would iterate through this Collection using its iterator and, for each
Object obtained, act like a setObject has been used with that Object and the
JAVA TYPE as an argument.

The Types.OTHER is used for database specific SQL types or for Dynamic Data
Access support. As the Collection class is not a data type there is no chance
of confusion.

It seems that Dima's idea can indeed work.

Regards,
Fernando

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Folks, it is getting hard to follow this thread which has actually forked into 3
or more subjects.

Lets please use the newsgroups convention of changing the subject line (making
the Re: into a Was: between square brackets).

Thanks a lot.

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Well, looks like Dima's problem can be solved after all while still
being standards compliant in the standard cases :-)
Still, the individual data types should be validated and correctly
escaped. If it counts at all, I would vote with this approach.
Just one more question Dima, how will you at runtime that the current
driver supports this functionality ? (for that matter, how do you know
now ?)

Cheers,
Csaba.

> Oliver,
>
> I think Dima is arguing that Collection could be treated as an special case
> where it indicates a list of something instead of a single something.
>
> So, we would iterate through this Collection using its iterator and, for each
> Object obtained, act like a setObject has been used with that Object and the
> JAVA TYPE as an argument.
>
> The Types.OTHER is used for database specific SQL types or for Dynamic Data
> Access support. As the Collection class is not a data type there is no chance
> of confusion.
>
> It seems that Dima's idea can indeed work.
>
> Regards,
> Fernando

On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:

> I think Dima is arguing that Collection could be treated as an special case
> where it indicates a list of something instead of a single something.
>
> So, we would iterate through this Collection using its iterator and, for
> each Object obtained, act like a setObject has been used with that Object
> and the JAVA TYPE as an argument.
>
> The Types.OTHER is used for database specific SQL types or for Dynamic Data
> Access support. As the Collection class is not a data type there is no
> chance of confusion.

Ya, I understand. My main objection is that setObject(n, object,
Types.INTEGER), in all other cases, means "interpret object as an integer,
or fail if it can't be cast in that way".

-O

Csaba Nagy wrote:

>Well, looks like Dima's problem can be solved after all while still
>being standards compliant in the standard cases :-)
>Still, the individual data types should be validated and correctly
>escaped. If it counts at all, I would vote with this approach.
>Just one more question Dima, how will you at runtime that the current
>driver supports this functionality ? (for that matter, how do you know
>now ?)
>
I am not sure what you mean by 'runtime' in this context.

*At runtime* i just need to know *which* driver I am using. There is, of
course, still work to be done at the implementation stage to figure out
and implement the proper way of dealing with that driver. So, the way I
know is same as usual - by looking at the source and/or documentation of
the driver.

What I am avoiding is direct references to the vendor-specific
classes/packages (like org.postgresql.*), because, if I had those, the
code would not compile unless I have all the drivers available at
compile time. That's what I am trying to avoid.

I understand that I still have to know specifics of the drivers I
support, and have driver-dependent logic in the code, that uses the
right way to deal with the driver at runtime. But I am trying to keep
that logic independent from the actual driver implementation, so that it
does not require the driver to compile.

Dima

>
>Cheers,
>Csaba.
>
>
>
>
>>Oliver,
>>
>>I think Dima is arguing that Collection could be treated as an special case
>>where it indicates a list of something instead of a single something.
>>
>>So, we would iterate through this Collection using its iterator and, for each
>>Object obtained, act like a setObject has been used with that Object and the
>>JAVA TYPE as an argument.
>>
>>The Types.OTHER is used for database specific SQL types or for Dynamic Data
>>Access support. As the Collection class is not a data type there is no chance
>>of confusion.
>>
>>It seems that Dima's idea can indeed work.
>>
>>Regards,
>>Fernando
>>
>>
>
>
>
>---------------------------(end of broadcast)---------------------------
>TIP 8: explain analyze is your friend
>
>

> >Just one more question Dima, how will you at runtime that the current
> >driver supports this functionality ? (for that matter, how do you know
> >now ?)
> >
> I am not sure what you mean by 'runtime' in this context.

I mean how does your code decide if this or that functionality is
available or not ? You can only make sure about that by trying out each
driver version you support for some functionality.

For example if you have written code which tests for postgres driver and
does not check the version of that driver, then after this change your
runtime code will not work anymore with new postgres drivers, because it
falsely presumes they support setting a list of integers as a parameter
using the setObject(x, String, Types.INTEGER) workaround.

On the other hand if your code does check version numbers, then indeed
you have to have some kind of list for each such feature of the drivers
supporting it, with the versions which support it, and you have to
follow when they stop doing it, etc. And you have to hard-code this
information in the application code so it has it at runtime, to be able
to make those decisions...

I guess this is a lot of maintenance work...

I was just wondering how you manage it.

Cheers,
Csaba.

On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver Jowett wrote:
> On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:
>
> > I think Dima is arguing that Collection could be treated as an special case
> > where it indicates a list of something instead of a single something.
> >
> > So, we would iterate through this Collection using its iterator and, for
> > each Object obtained, act like a setObject has been used with that Object
> > and the JAVA TYPE as an argument.
> >
> > The Types.OTHER is used for database specific SQL types or for Dynamic Data
> > Access support. As the Collection class is not a data type there is no
> > chance of confusion.
>
> Ya, I understand. My main objection is that setObject(n, object,
> Types.INTEGER), in all other cases, means "interpret object as an integer,
> or fail if it can't be cast in that way".

Also.. what would we do with this object?

public class AnnoyingObject implements java.util.Collection, java.sql.Array {
// ...
}

then setObject(n, new AnnoyingObject(), Types.ARRAY);

Is that an Array, or an IN clause of Arrays? :)

(Array is the obvious candidate for also being a Collection, but potentially
you could do it with other types too)

-O

>
>
>I mean how does your code decide if this or that functionality is
>available or not ? You can only make sure about that by trying out each
>driver version you support for some functionality.
>
Yup. That's what I do.

>
>For example if you have written code which tests for postgres driver and
>does not check the version of that driver, then after this change your
>runtime code will not work anymore with new postgres drivers, because it
>falsely presumes they support setting a list of integers as a parameter
>using the setObject(x, String, Types.INTEGER) workaround.
>
I know :-(
That's exactly why I was trying so hard to argue against it.
I guess, I am going to have to tell my customers that postgres driver
later than 7.3 is not supported...
or I'll have to modify the new driver to put the original functionality
back, and let them download my version :-(

>
>On the other hand if your code does check version numbers, then indeed
>you have to have some kind of list for each such feature of the drivers
>supporting it, with the versions which support it, and you have to
>follow when they stop doing it, etc. And you have to hard-code this
>information in the application code so it has it at runtime, to be able
>to make those decisions...
>
It is *really really* (I can't repeat it enough times) uncommon for
database vendors (other that postgres obviously - sigh) to make
backwards incompatible changes to their software. In other words, I
don't really need any 'list of versions' - just the earliest supported
version should be enough.

>
>I guess this is a lot of maintenance work...
>
>
It is not really that bad... not until I run into this
backwards-incompatible api changes...
It happened in the past once or twice (before I got here, so I don't
know the details), and they even had to drop support of one of the
databases because of that :-(

Dima

>
>
>Also.. what would we do with this object?
>
>public class AnnoyingObject implements java.util.Collection, java.sql.Array {
> // ...
>}
>
>then setObject(n, new AnnoyingObject(), Types.ARRAY);
>
>Is that an Array, or an IN clause of Arrays? :)
>
>(Array is the obvious candidate for also being a Collection, but potentially
>you could do it with other types too)
>
>
>
java.sql.Array is an ARRAY.
I don't think there is any ambiguity here, as it is the only reason I
can imagine for somebody to implement a sql.Array is to pass an ARRAY
into a PreparedStatement.

If they wanted a set of arrays, they would have to pass in a Collection,
containing java.sql.Arrays as elements...

Dima

Oliver Jowett wrote:> On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver Jowett wrote:
>
>>On Mon, Jul 21, 2003 at 11:26:11AM -0400, Fernando Nasser wrote:
>>
>>
>>>I think Dima is arguing that Collection could be treated as an special case
>>>where it indicates a list of something instead of a single something.
>>>
>>>So, we would iterate through this Collection using its iterator and, for
>>>each Object obtained, act like a setObject has been used with that Object
>>>and the JAVA TYPE as an argument.
>>>
>>>The Types.OTHER is used for database specific SQL types or for Dynamic Data
>>>Access support. As the Collection class is not a data type there is no
>>>chance of confusion.
>>
>>Ya, I understand. My main objection is that setObject(n, object,
>>Types.INTEGER), in all other cases, means "interpret object as an integer,
>>or fail if it can't be cast in that way".
>
>
> Also.. what would we do with this object?
>
> public class AnnoyingObject implements java.util.Collection, java.sql.Array {
> // ...
> }
>
> then setObject(n, new AnnoyingObject(), Types.ARRAY);
>
> Is that an Array, or an IN clause of Arrays? :)
>
> (Array is the obvious candidate for also being a Collection, but potentially
> you could do it with other types too)
>

I am not sure if this is an useful or usual Java class at all, but if you want
to pass a list of this AnnoyingObject you can always create a Collection of such
objects (like an ArrayList).

With setObject, if the specified type is Array and the passed type is an Array
of some sort we have to honor that. So, in the obscure case where someone wants
to set a list of Arrays they will have to add the Arrays to a Collection (that
is not an Array itself).

But you are right, to my pseudo code we do have to check the Array to ARRAY case
before assuming that the list is what the programmer wants.

Also, we may limit this behavior with Collections to the IN clause only. Where
else could we need lists to replace the '?' ?

--
Fernando Nasser
Red Hat - Toronto E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

>
> Also, we may limit this behavior with Collections to the IN clause
> only. Where else could we need lists to replace the '?' ?
>
Arrays would be nice :-) (so that the user would not have to implement
his own java.sql.Array, and wrap his collections into it to be able to
set array parameters).
Informix supports that, for example (but it has to be ArrayList for some
reason - not just any Collection), oracle does too IIRC...

Dima

Peter Kovacs wrote:
> That's, then, even simpler that I originally thought. The only thing to
> be done is to make using "real" prepared statements the default
> behaviour of the PreparedStatement instances, is not it?
>

Yes and that is the plan. But it is a lot of work.

--Barry

> Fernando Nasser wrote:
>
>> Peter Kovacs wrote:> I think that the simplest thing would be to have
>> an option in the
>>
>>> backend to disable processing of multiple statements in one query --
>>> i.e. disallow the use of ';' as a separator of statements. I am not
>>> sure why this feature (multiple statments in one query) is there
>>> anyway. "Reduce network roundtrips" is the usual reply, but, then,
>>> what is the purpose of stored procedures (functions in PostgreSQL)?
>>>
>>
>> I don't think the backend maintainers will like that. And btw we
>> don't need this at all.
>>
>> 1) There is no risk of SQL injection with "real" (or "server side")
>> prepared statements.
>>
>> 2) With the proper implementation of the client side prepared
>> statements emulation the injected statements will not go through,
>> because:
>>
>> a) all non-string results are properly quoted and generated from
>> primary types.
>>
>> b) String type contents are quoted and have its quotes escaped.
>>
>>> In my perception, JDBC is meant to be a client side extension of the
>>> server - bridge for Java clients to use the server (which in our case
>>> is the right and honorable PostgreSQL). Prepared statements is a
>>> mechanism to indicate the server that more query of this kind is
>>> likely to come so the plan of the query should be kept ready to be
>>> used again. That is what meant by PreparedStatement in the JDBC
>>> driver. I find the concept of "client side prepared statements"
>>> pretty weird.
>>>
>>
>> It was added before the server had prepared statements so the
>> applications could still use PreparedStatements and the getXXX and
>> setXXX methods instead of recreating strings with queries all the
>> time. Some of the applications run with other databases as well.
>>
>>> From this perspective, the whole wrestling with "drop table..." and
>>> similar risks seem farily vain to me. At least, the driver is not the
>>> place to solve this kind of security problems which basically exist
>>> due to the wya the server behaves. Instead, the question should be
>>> asked: is the behaviour of the server optimal?. Do we need this
>>> feature (processing multiple, semi-colon separated statements)?
>>> Should not this feature be eventually optional?
>>>
>>
>> No SQL injection is possible with the backend implementation of
>> prepared statements (with the V3 protocol, at least).
>>
>>
>>
>
>

On Mon, Jul 21, 2003 at 12:33:40PM -0400, Dmitry Tkach wrote:
> >
> >
> >Also.. what would we do with this object?
> >
> >public class AnnoyingObject implements java.util.Collection,
> >java.sql.Array {
> > // ...
> >}
> >
> >then setObject(n, new AnnoyingObject(), Types.ARRAY);
> >
> >Is that an Array, or an IN clause of Arrays? :)
> >
> >(Array is the obvious candidate for also being a Collection, but
> >potentially
> >you could do it with other types too)
> >
> >
> >
> java.sql.Array is an ARRAY.
> I don't think there is any ambiguity here, as it is the only reason I
> can imagine for somebody to implement a sql.Array is to pass an ARRAY
> into a PreparedStatement.
>
>
> If they wanted a set of arrays, they would have to pass in a Collection,
> containing java.sql.Arrays as elements...

Reread the class declaration, AnnoyingObject is both an Array and a
Collection.

-O

On Mon, Jul 21, 2003 at 01:51:41PM -0400, Fernando Nasser wrote:
> Oliver Jowett wrote:> On Tue, Jul 22, 2003 at 03:47:49AM +1200, Oliver
> Jowett wrote:

> >Also.. what would we do with this object?
> >
> >public class AnnoyingObject implements java.util.Collection,
> >java.sql.Array {
> > // ...
> >}
> >
> >then setObject(n, new AnnoyingObject(), Types.ARRAY);
> >
> >Is that an Array, or an IN clause of Arrays? :)
> >
> >(Array is the obvious candidate for also being a Collection, but
> >potentially
> >you could do it with other types too)
> >
>
> I am not sure if this is an useful or usual Java class at all, but if you
> want to pass a list of this AnnoyingObject you can always create a
> Collection of such objects (like an ArrayList).

Um, no, that's not my point.

Consider this (more realistic) example:

public class MutableArray extends ArrayList implements java.sql.Array {
// implementation of java.sql.Array in terms of ArrayList methods
}

MutableArray myarray = new MutableArray();
myarray.add("abcd");
myarray.add("efgh");

stmt.setArray(1, myarray); // This sets param 1 as an array of strings
stmt.setObject(1, myarray); // but what does this do?
stmt.setObject(1, myarray, Types.ARRAY); // and this?
stmt.setObject(1, myarray, Types.VARCHAR); // and this?

Yes, you can avoid this by using composition not inheritance. But this is a
very fragile and nonobvious way for setObject to behave. Adding an extra,
commonly used, non-JDBC, non-Postgresql interface to an existing class
shouldn't cause large changes to how the driver treats it!

> With setObject, if the specified type is Array and the passed type is an
> Array of some sort we have to honor that. So, in the obscure case where
> someone wants to set a list of Arrays they will have to add the Arrays to a
> Collection (that is not an Array itself).

So the extension becomes "If you pass a Collection.. unless the Collection
is also an Array and you specify Types.ARRAY.. or it's also a Blob and you
specify Types.BLOB.. or ...". This is getting messy.

> Also, we may limit this behavior with Collections to the IN clause only.
> Where else could we need lists to replace the '?' ?

Ideally, we want an interface where the API user can provide the JDBC driver
with enough information to say "this is definitely an IN clause parameter"
without having to inspect the query. Otherwise you have the situation where
the same setObject() call expands differently depending on parameter context,
which is pretty nasty.

-O

I have applied a fix for the reported SQL injection vulnerability in
setObject for both 7.3 and 7.4. I have also places new builds with this
bugfix on the jdbc.postgresql.org website.

For the record, the vulnerability was limited to the
setObject(int,Object,int) method where Object is a String and the type
is a numeric type (i.e. SqlTypes.INTEGER, SqlTypes.LONG, etc)

Given the ongoing discussion that this SQL injection vulnerability has
caused, I decided not to apply the below patch from Kim and instead
fixed the problem in a different way. The fix essentially applies the
regular escaping done for setString to appropriate values passed to
setObject. It does not however add quotes to the value. Thus existing
uses of setObject for in clause and array type values will still
continue to work.

I didn't want to break backward compatibility in a patch to 7.3 and
didn't want to change the functionality in 7.4 until the current
discussions come to a conclusion.

thanks,
--Barry

PS. I have not yet uploaded new jdbc1 builds to the website. As I have
recently upgraded to RH9, my jdk1.1 environment no longer works and I
will need to do this build on a different machine tomorrow.

Kim Ho wrote:
> To speed things up a bit, since the regoutParam patch is not likely to
> be approved anytime soon.
>
> This patch
> - adds single quotes for numbers in setObject and also setInt/Byte/etc.
> - Improves getInt/Long when you may have parser errors if you're too
> close to Integer.MIN_VALUE or Integer.MAX_VALUE. Thanks to Fujitsu.
> - Improves radix point handling when using setObject to an integer
> parameter while passing in a float. This is especially important in
> callable statements.
>
> Cheers,
>
> Kim
>
> On Fri, 2003-07-18 at 12:51, Fernando Nasser wrote:
>
>>Barry Lind wrote:
>>
>>>Dmitry,
>>>
>>>That is a bug. Thanks for pointing it out. Anyone care to submit a patch?
>>>
>>
>>Kim's patch fixes this. It is pending approval.
>>
>>
>>
>>--
>>Fernando Nasser
>>Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
>>2323 Yonge Street, Suite #300
>>Toronto, Ontario M4P 2C9
>>
>>
>>---------------------------(end of broadcast)---------------------------
>>TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org
>
>
>
> ------------------------------------------------------------------------
>
> ? temp.diff
> Index: org/postgresql/jdbc1/AbstractJdbc1ResultSet.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1ResultSet.java,v
> retrieving revision 1.13
> diff -c -p -r1.13 AbstractJdbc1ResultSet.java
> *** org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 30 Jun 2003 21:10:55 -0000 1.13
> --- org/postgresql/jdbc1/AbstractJdbc1ResultSet.java 18 Jul 2003 17:02:20 -0000
> *************** public abstract class AbstractJdbc1Resul
> *** 805,811 ****
> try
> {
> s = s.trim();
> ! return Integer.parseInt(s);
> }
> catch (NumberFormatException e)
> {
> --- 805,811 ----
> try
> {
> s = s.trim();
> ! return Float.valueOf(s).intValue();
> }
> catch (NumberFormatException e)
> {
> *************** public abstract class AbstractJdbc1Resul
> *** 822,828 ****
> try
> {
> s = s.trim();
> ! return Long.parseLong(s);
> }
> catch (NumberFormatException e)
> {
> --- 822,828 ----
> try
> {
> s = s.trim();
> ! return Double.valueOf(s).longValue();
> }
> catch (NumberFormatException e)
> {
> Index: org/postgresql/jdbc1/AbstractJdbc1Statement.java
> ===================================================================
> RCS file: /projects/cvsroot/pgsql-server/src/interfaces/jdbc/org/postgresql/jdbc1/AbstractJdbc1Statement.java,v
> retrieving revision 1.27
> diff -c -p -r1.27 AbstractJdbc1Statement.java
> *** org/postgresql/jdbc1/AbstractJdbc1Statement.java 9 Jul 2003 05:12:04 -0000 1.27
> --- org/postgresql/jdbc1/AbstractJdbc1Statement.java 18 Jul 2003 17:02:22 -0000
> *************** public abstract class AbstractJdbc1State
> *** 920,926 ****
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
> ! bind(parameterIndex, Integer.toString(x), PG_TEXT);
> }
>
> /*
> --- 920,926 ----
> */
> public void setByte(int parameterIndex, byte x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_TEXT);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 933,939 ****
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
> ! bind(parameterIndex, Integer.toString(x), PG_INT2);
> }
>
> /*
> --- 933,939 ----
> */
> public void setShort(int parameterIndex, short x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Integer.toString(x) + "'" , PG_INT2);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 946,952 ****
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
> ! bind(parameterIndex, Integer.toString(x), PG_INTEGER);
> }
>
> /*
> --- 946,952 ----
> */
> public void setInt(int parameterIndex, int x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Integer.toString(x) + "'", PG_INTEGER);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 959,965 ****
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
> ! bind(parameterIndex, Long.toString(x), PG_INT8);
> }
>
> /*
> --- 959,965 ----
> */
> public void setLong(int parameterIndex, long x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Long.toString(x) + "'", PG_INT8);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 972,978 ****
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
> ! bind(parameterIndex, Float.toString(x), PG_FLOAT);
> }
>
> /*
> --- 972,978 ----
> */
> public void setFloat(int parameterIndex, float x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Float.toString(x) + "'", PG_FLOAT);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 985,991 ****
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
> ! bind(parameterIndex, Double.toString(x), PG_DOUBLE);
> }
>
> /*
> --- 985,991 ----
> */
> public void setDouble(int parameterIndex, double x) throws SQLException
> {
> ! bind(parameterIndex, "'" + Double.toString(x) + "'", PG_DOUBLE);
> }
>
> /*
> *************** public abstract class AbstractJdbc1State
> *** 1003,1009 ****
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
> ! bind(parameterIndex, x.toString(), PG_NUMERIC);
> }
> }
>
> --- 1003,1009 ----
> setNull(parameterIndex, Types.DECIMAL);
> else
> {
> ! bind(parameterIndex, "'" + x.toString() + "'", PG_NUMERIC);
> }
> }
>
> *************** public abstract class AbstractJdbc1State
> *** 1464,1486 ****
> switch (targetSqlType)
> {
> case Types.INTEGER:
> - if (x instanceof Boolean)
> - bind(parameterIndex,((Boolean)x).booleanValue() ? "1" :"0", PG_BOOLEAN);
> - else
> - bind(parameterIndex, x.toString(), PG_INTEGER);
> - break;
> case Types.TINYINT:
> case Types.SMALLINT:
> case Types.BIGINT:
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
> ! if (x instanceof Boolean)
> ! bind(parameterIndex, ((Boolean)x).booleanValue() ? "1" : "0", PG_BOOLEAN);
> ! else
> ! bind(parameterIndex, x.toString(), PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
> --- 1464,1484 ----
> switch (targetSqlType)
> {
> case Types.INTEGER:
> case Types.TINYINT:
> case Types.SMALLINT:
> + x = removeRadix(x,Types.INTEGER);
> + bindNumber(parameterIndex,x,PG_INTEGER);
> + break;
> case Types.BIGINT:
> + x = removeRadix(x,Types.BIGINT);
> + bindNumber(parameterIndex,x,PG_INT8);
> + break;
> case Types.REAL:
> case Types.FLOAT:
> case Types.DOUBLE:
> case Types.DECIMAL:
> case Types.NUMERIC:
> ! bindNumber(parameterIndex,x,PG_NUMERIC);
> break;
> case Types.CHAR:
> case Types.VARCHAR:
> *************** public abstract class AbstractJdbc1State
> *** 2026,2031 ****
> --- 2024,2056 ----
> if (parameterIndex != 1)
> throw new PSQLException("postgresql.call.noinout");
> }
> +
> + private void bindNumber(int parameterIndex, Object x, String pgtype) throws SQLException
> + {
> + if (x instanceof Boolean)
> + bind(parameterIndex,((Boolean)x).booleanValue() ? "'1'" :"'0'", pgtype);
> + else
> + bind(parameterIndex, "'"+x.toString()+"'", pgtype);
> + }
> +
> +
> + private Object removeRadix(Object x, int sqlType)
> + {
> + if (x.toString().indexOf(".")>0)
> + {
> + switch (sqlType)
> + {
> + case Types.BIGINT:
> + x = String.valueOf(Double.valueOf(x.toString()).longValue());
> + break;
> + default:
> + x = String.valueOf(Float.valueOf(x.toString()).intValue());
> + break;
> + }
> + }
> + return x;
> + }
> +
>
>
>
>
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> message can get through to the mailing list cleanly

On Mon, 21 Jul 2003, Peter Kovacs wrote:

> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements. I am not sure
> why this feature (multiple statments in one query) is there anyway.
> "Reduce network roundtrips" is the usual reply, but, then, what is the
> purpose of stored procedures (functions in PostgreSQL)?
>

> From this perspective, the whole wrestling with "drop table..." and
> similar risks seem farily vain to me. At least, the driver is not the
> place to solve this kind of security problems which basically exist due
> to the wya the server behaves. Instead, the question should be asked: is
> the behaviour of the server optimal?. Do we need this feature
> (processing multiple, semi-colon separated statements)? Should not this
> feature be eventually optional?

The second statement type of attack is just one variant. Consider a query
that displayed a list of your orders. SELECT * FROM orders WHERE
userid='username'. Suppose I substituted a username of username' OR
true OR userid='. This is another injection attack that does not
require the backend to support multiple statements per query.

Kris Jurka

Peter Kovacs <peter(dot)kovacs(at)siemens(dot)com> writes:
> I think that the simplest thing would be to have an option in the
> backend to disable processing of multiple statements in one query --
> i.e. disallow the use of ';' as a separator of statements.

FWIW, the new "extended query" protocol has exactly such a restriction.
However that hardly excuses any sloppiness in allowing
non-syntax-checked parameter values through. Consider changing
"WHERE x < ?" to
"WHERE x < 42 AND my_function_with_interesting_side_effects()"

No semicolons in sight, but I can still clean out your bank balance ;-)

regards, tom lane

On Mon, Jul 21, 2003 at 10:49:14PM -0700, Barry Lind wrote:

> Given the ongoing discussion that this SQL injection vulnerability has
> caused, I decided not to apply the below patch from Kim and instead
> fixed the problem in a different way. The fix essentially applies the
> regular escaping done for setString to appropriate values passed to
> setObject. It does not however add quotes to the value. Thus existing
> uses of setObject for in clause and array type values will still
> continue to work.

I haven't looked at the updated tree yet, but from your description won't
this break code that does something like this? :

stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);

-O

Tom Lane wrote:

>Peter Kovacs <peter(dot)kovacs(at)siemens(dot)com> writes:
>
>
>>I think that the simplest thing would be to have an option in the
>>backend to disable processing of multiple statements in one query --
>>i.e. disallow the use of ';' as a separator of statements.
>>
>>
>
>FWIW, the new "extended query" protocol has exactly such a restriction.
>However that hardly excuses any sloppiness in allowing
>non-syntax-checked parameter values through. Consider changing
>"WHERE x < ?" to
>"WHERE x < 42 AND my_function_with_interesting_side_effects()"
>
>No semicolons in sight, but I can still clean out your bank balance ;-)
>
...and it would serve me right :(.

BTW, I presume that one can deny a user the right to create stored
procedures in PostgreSQL. Anyway, I now recognize that the issue is more
complicated than allowing';'.

Regards,
Peter

>
> regards, tom lane
>
>

On 21/07/2003 18:51 Fernando Nasser wrote:
> Also, we may limit this behavior with Collections to the IN clause
> only. Where else could we need lists to replace the '?' ?

Nowhere. Not even with an IN clause. If the programmer needs IN(1,2,3,4,5)
then he must write IN(?,?,?,?,?) in his prepare string. That's the way
JDBC works. Period. Acceptance of any other behaviour is un-professional
and against the standards. As you said yourself, neither Oracle nor DB2
support this behavior. Neither should PostgreSQL.

--
Paul Thomas
+------------------------------+---------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for the Smaller
Business |
| Computer Consultants |
http://www.thomas-micro-systems-ltd.co.uk |
+------------------------------+---------------------------------------------+

Well, that's your opinion, calm down :-)
Anyway, I really think the solution of add various parameter marks ("?") and fill the ones you don't use with nulls is rather awful. There is a database that provides another solution for that?

On Tue, 22 Jul 2003 09:34:10 +0100
Paul Thomas <paul(at)tmsl(dot)demon(dot)co(dot)uk> wrote:

>
> On 21/07/2003 18:51 Fernando Nasser wrote:
> > Also, we may limit this behavior with Collections to the IN clause
> > only. Where else could we need lists to replace the '?' ?
>
> Nowhere. Not even with an IN clause. If the programmer needs IN(1,2,3,4,5)
> then he must write IN(?,?,?,?,?) in his prepare string. That's the way
> JDBC works. Period. Acceptance of any other behaviour is un-professional
> and against the standards. As you said yourself, neither Oracle nor DB2
> support this behavior. Neither should PostgreSQL.
>
>
> --
> Paul Thomas
> +------------------------------+---------------------------------------------+
> | Thomas Micro Systems Limited | Software Solutions for the Smaller
> Business |
> | Computer Consultants |
> http://www.thomas-micro-systems-ltd.co.uk |
> +------------------------------+---------------------------------------------+
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match

--

/~\ The ASCII Felipe Schnack (felipes(at)ritterdosreis(dot)br)
\ / Ribbon Campaign Analista de Sistemas
X Against HTML Cel.: 51-91287530
/ \ Email! Linux Counter #281893

Centro Universitário Ritter dos Reis
http://www.ritterdosreis.br
ritter(at)ritterdosreis(dot)br
Fone: 51-32303341

Paul Thomas wrote:
>
> On 21/07/2003 18:51 Fernando Nasser wrote:
>
>> Also, we may limit this behavior with Collections to the IN clause
>> only. Where else could we need lists to replace the '?' ?
>
>
> Nowhere. Not even with an IN clause. If the programmer needs
> IN(1,2,3,4,5) then he must write IN(?,?,?,?,?) in his prepare string.
> That's the way JDBC works. Period. Acceptance of any other behaviour is
> un-professional and against the standards. As you said yourself, neither
> Oracle nor DB2 support this behavior. Neither should PostgreSQL.
>

Well, I was just informed that the 7.4 backend supports an IN list which
is filled with a PostgreSQL array. As the syntax requires the
parenthesis to be in place (it only fills the list itself) there is no
ambiguity.

Its support is limited on 7.4 (the optimized is not aware of it and
generates a crappy plan) but one may consider improving it for 7.5.

--
Fernando Nasser
Red Hat Canada Ltd. E-Mail: fnasser(at)redhat(dot)com
2323 Yonge Street, Suite #300
Toronto, Ontario M4P 2C9

Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> ... won't this break code that does something like this? :

> stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
> stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);

Code that does that is just going to have to break. We should try to
provide equivalent functionality in a less unsafe fashion; but
backwards compatibility with code that is exploiting a security hole
is not an option.

regards, tom lane

On Tue, Jul 22, 2003 at 09:33:53AM -0400, Tom Lane wrote:
> Oliver Jowett <oliver(at)opencloud(dot)com> writes:
> > ... won't this break code that does something like this? :
>
> > stmt = conn.prepareStatement("SELECT * FROM table WHERE string_key IN ?");
> > stmt.setObject(1, "('a', 'b', 'c')", Types.NUMERIC);
>
> Code that does that is just going to have to break. We should try to
> provide equivalent functionality in a less unsafe fashion; but
> backwards compatibility with code that is exploiting a security hole
> is not an option.

I agree; since we can't remain backwards-compatible in all cases, is it
worth doing the odd halfway-escaped thing for the sake of the remaining
cases?

-O