Re: SQL_CURSOR_TYPE prepare execute issue

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=0 in 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(at)vmware(dot)com]
Sent: 13 January 2015 23:00
To: Faith, Jeremy; pgsql-odbc(at)postgresql(dot)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 checking done 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(and safer):-
> 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 are not the intended recipient of this e-mail, you are hereby notified that you must not disseminate, copy or take any action in respect of any information contained in it. If you have received this e-mail in error, please notify the sender immediately by e-mail and immediately destroy this e-mail and its attachments.