Re: SQL_CURSOR_TYPE prepare execute issue - Mailing list pgsql-odbc

From Faith, Jeremy
Subject Re: SQL_CURSOR_TYPE prepare execute issue
Date
Msg-id 55BAADA01D8C504993894EAEA7517CF80D08BD@003FCH1MPN2-002.003f.mgd2.msft.net
Whole thread Raw
In response to Re: SQL_CURSOR_TYPE prepare execute issue  (Heikki Linnakangas <hlinnakangas@vmware.com>)
Responses Re: SQL_CURSOR_TYPE prepare execute issue  (Heikki Linnakangas <hlinnakangas@vmware.com>)
Time for a new release? (was Re: SQL_CURSOR_TYPE prepare execute issue)  (Heikki Linnakangas <hlinnakangas@vmware.com>)
List pgsql-odbc
Hi,

Thanks for the fix.

I think the test code should call
  SQLSetStmtOption(hstmt,SQL_CURSOR_TYPE,SQL_CURSOR_STATIC);
after SQLAllocHandle. As otherwise even with the 9.03.03 version of the ODBC driver the correct error occurs unless
UseServerSidePrepare=0in the odbc.ini file. That is I am not sure the changed code is being used by the test as it is. 

I have had a quick look over the change and it looks ok to me. Something of a clean up and simplification as well.
If I understand it correctly, the only things that don't get quoted are SQL_INTEGER and SQL_SMALLINT that pass the new
valid_int_literal()test. 
The only thing I can see that could pass that test and not be a valid integer would be a single minus char i.e. "-"
not sure if there is anyway that could be vulnerable though.

Any idea when a new release of the driver is likely?
I know a big change to use libpq has occurred recently so I guess it could be a while.

Regards,
Jeremy Faith

________________________________________
From: Heikki Linnakangas [hlinnakangas@vmware.com]
Sent: 13 January 2015 23:00
To: Faith, Jeremy; pgsql-odbc@postgresql.org
Subject: Re: [ODBC] SQL_CURSOR_TYPE prepare execute issue

On 01/13/2015 07:57 PM, Faith, Jeremy wrote:
> Hi,
>
> With PosgreSQL 9.3.5 and pgsqlodbc 9.03.03
>
> I have found an issue with setting cursor type to anything other than the default(SQL_CURSOR_FORWARD_ONLY) and the
checkingdone on prepare/execute parameters. 
>
> In particular with regards to protection from SQL injection attacks.
>
> Say for example there is a table
>    create table t1(c1 integer);
>
> When following is run
>    SQLSMALLINT dec_digits;
>    SQLUINTEGER col_size;
>    SQLHSTMT stmt;
>    char buf[100];
>
>    //... not showing con setup
>    SQLAllocHandle(SQL_HANDLE_STMT,con->sqlcon,&stmt);
>
>    SQLSetStmtOption(stmt,SQL_CURSOR_TYPE,SQL_CURSOR_STATIC);
>    SQLPrepare(stmt,(SQLCHAR *)"select * from t1 where c1=?",SQL_NTS);
>
>    sprintf(buf,"%s","1 and zzz='dummy'"); //i.e. injection attempt
> //this could be much worse e.g. "1; drop table personnel;"
>    len=strlen(buf);
>    col_size=10;
>    dec_digits=-1;
>    SQLBindParameter(stmt,1,SQL_PARAM_INPUT,SQL_C_CHAR,SQL_INTEGER,col_size,
>      dec_digits,buf,0,&len);
>
>    SQLExecute(stmt);
>
> The SQLExecute will fail(as it should) but with the cursor set to SQL_CURSOR_STATIC then the error is:-
>    stmt native_err=7 [42703]ERROR: column "zzz" does not exist;
>
> Whereas if the cursor type is not set(and is thus the default SQL_CURSOR_FORWARD_ONLY) then the error is much
better(andsafer):- 
>    stmt native_err=7 [22P02]ERROR: invalid input syntax for integer: "1 and zzz='dummy' ";
>
> Also if in the odbc.ini file
>    UseServerSidePrepare  = 0
> for the connection then error is always: column "zzz" does not exist, regardless of the cursor type.

Hmm. The driver blindly assumes that if parameter is bound as
SQL_INTEGER or SQL_SMALLINT, it doesn't require quoting. But clearly
that's not true, if the passed parameter string is not a valid integer.

> I found this problem when testing with PHP odbc_prepare/odbc_execute
> functions as the odbc_prepare function sets the cursor type to
> SQL_CURSOR_STATIC(by default though it can be overridden by setting
> odbc.default_cursortype = SQL_CURSOR_FORWARD_ONLY in php.ini). PHP
> needs UseServerSidePrepare=1 as it also calls SQLDescribeParam()
> which fails if UseServerSidePrepare=0.
>
> The question is why does changing the cursor type change the way the
> parameters are bound? Can anything be done to fix this?

I just pushed a fix and a regression test to the git repository. Thanks
for the report!

- Heikki

Tyco Safety Products/CEM Systems Ltd.

________________________________

This e-mail contains privileged and confidential information intended for the use of the addressees named above. If you
arenot the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any
actionin respect of any information contained in it. If you have received this e-mail in error, please notify the
senderimmediately by e-mail and immediately destroy this e-mail and its attachments. 


pgsql-odbc by date:

Previous
From: Heikki Linnakangas
Date:
Subject: Re: ODBC for PG 9.4.x?
Next
From: Heikki Linnakangas
Date:
Subject: Re: [issue?] SQLDriverConnect will clear existing connect attribute setting.