Fwd: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC breaks passing char buffer length at S

Fwd: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC breaks passing char buffer length at S

am 06.11.2006 20:46:15 von Peter Eisentraut

This is a multi-part MIME message sent by reportbug.


--Boundary-00=_IE5TF8ZFVtPVXNR
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

A Debian bug report ...

---------- Forwarded Message ----------

Subject: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC breaks
passing char buffer length at SQLExec time
Date: Monday 30 October 2006 17:00
From: Enrico Zini
To: Debian Bug Tracking System

Package: odbc-postgresql
Version: 1:08.01.0200-2
Severity: normal
Tags: patch

Hello,

thanks for maintaining odbc-postgresql.

In my package dballe (apt-get source dballe) I optimize some frequent
query by preparing the ODBC statement for them and binding the input
parameters, so that when I need the results I can just set some of the
bound variables and call SQLExecute.

This is an example of such variable bind code:

SQLBindParameter(res->istm, 3, SQL_PARAM_INPUT, SQL_C_CHAR,
SQL_CHAR, 0, 0, &(res->value), 0, &(res->value_ind));

Note that I'm passing a pointer to the variable that will hold the
string length as the last parameter, instead of using SQL_NTS.

Now, this call eventually ends up in this code, found at bind.c:118:

/* Data at exec macro only valid for C char/binary data */
if (pcbValue && (*pcbValue == SQL_DATA_AT_EXEC ||
*pcbValue <=
SQL_LEN_DATA_AT_EXEC_OFFSET)) apdopts->parameters[ipar].data_at_exec =
TRUE;
else
apdopts->parameters[ipar].data_at_exec = FALSE;

This code tries to dereferenciate the (still uninitialised) pointer I
pass it ( that &(res->value_ind) ) and fails. The test should be like
this instead:

/* Data at exec macro only valid for C char/binary data */
if (pcbValue && (pcbValue == SQL_DATA_AT_EXEC ||
pcbValue <=
SQL_LEN_DATA_AT_EXEC_OFFSET)) apdopts->parameters[ipar].data_at_exec =
TRUE;
else
apdopts->parameters[ipar].data_at_exec = FALSE;

ODBC has this dirty habit of passing negative integers as special
values for pointers, and SQL_DATA_AT_EXEC is one of those:

from /usr/include/sql.h:29:

#define SQL_NULL_DATA (-1)
#define SQL_DATA_AT_EXEC (-2)

same goes for the handling of SQL_LEN_DATA_AT_EXEC_OFFSET.

I regret I can't test the patch with code that uses SQL_DATA_AT_EXEC,
because that is an ODBC feature that I do not use. However, with the
patch my SQLBindParameter above works fine.


Ciao,

Enrico


-- System Information:
Debian Release: 4.0
APT prefers unstable
APT policy: (500, 'unstable')
Architecture: i386 (i686)
Shell: /bin/sh linked to /bin/bash
Kernel: Linux 2.6.18-1-686
Locale: LANG=it_IT.UTF-8, LC_CTYPE=it_IT.UTF-8 (charmap=UTF-8)

Versions of packages odbc-postgresql depends on:
ii libc6 2.3.6.ds1-7 GNU C Library: Shared
libraries ii libpq4 8.1.5-1 PostgreSQL C
client library ii odbcinst1debian1 2.2.11-13 Support
library and helper program

odbc-postgresql recommends no packages.

-- no debconf information

-------------------------------------------------------
--
Peter Eisentraut
http://developer.postgresql.org/~petere/

--Boundary-00=_IE5TF8ZFVtPVXNR
Content-Type: text/x-c;
charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="SQLBindParameter.patch"

diff -Naur psqlodbc-08.01.0200/bind.c psqlodbc-08.01.0200.enrico/bind.c
--- psqlodbc-08.01.0200/bind.c 2005-11-25 11:43:25.000000000 +0100
+++ psqlodbc-08.01.0200.enrico/bind.c 2006-10-30 16:44:45.644656335 +0100
@@ -116,8 +116,8 @@
if (pcbValue && apdopts->param_offset_ptr)
pcbValue += (*apdopts->param_offset_ptr >> 2);
/* Data at exec macro only valid for C char/binary data */
- if (pcbValue && (*pcbValue == SQL_DATA_AT_EXEC ||
- *pcbValue <= SQL_LEN_DATA_AT_EXEC_OFFSET))
+ if (pcbValue && (pcbValue == SQL_DATA_AT_EXEC ||
+ pcbValue <= SQL_LEN_DATA_AT_EXEC_OFFSET))
apdopts->parameters[ipar].data_at_exec = TRUE;
else
apdopts->parameters[ipar].data_at_exec = FALSE;

--Boundary-00=_IE5TF8ZFVtPVXNR
Content-Type: text/plain
Content-Disposition: inline
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


---------------------------(end of broadcast)---------------------------
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate

--Boundary-00=_IE5TF8ZFVtPVXNR--

Re: Fwd: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC

am 07.11.2006 02:10:18 von Hiroshi Inoue

Peter Eisentraut wrote:
> A Debian bug report ...

Enrico is right at the point that the evaluation of *pcbValue here is
dangerous and has no meaning. So in my version I already invalidated
the code(#ifdef NOT_USED). Unfortunately his patch seems to have no
meaning.

regards,
Hiroshi Inoue

> ---------- Forwarded Message ----------
>
> Subject: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC breaks
> passing char buffer length at SQLExec time
> Date: Monday 30 October 2006 17:00
> From: Enrico Zini
> To: Debian Bug Tracking System
>
> Package: odbc-postgresql
> Version: 1:08.01.0200-2
> Severity: normal
> Tags: patch
>
> Hello,
>
> thanks for maintaining odbc-postgresql.
>
> In my package dballe (apt-get source dballe) I optimize some frequent
> query by preparing the ODBC statement for them and binding the input
> parameters, so that when I need the results I can just set some of the
> bound variables and call SQLExecute.
>
> This is an example of such variable bind code:
>
> SQLBindParameter(res->istm, 3, SQL_PARAM_INPUT, SQL_C_CHAR,
> SQL_CHAR, 0, 0, &(res->value), 0, &(res->value_ind));
>
> Note that I'm passing a pointer to the variable that will hold the
> string length as the last parameter, instead of using SQL_NTS.
>
> Now, this call eventually ends up in this code, found at bind.c:118:
>
> /* Data at exec macro only valid for C char/binary data */
> if (pcbValue && (*pcbValue == SQL_DATA_AT_EXEC ||
> *pcbValue <=
> SQL_LEN_DATA_AT_EXEC_OFFSET)) apdopts->parameters[ipar].data_at_exec =
> TRUE;
> else
> apdopts->parameters[ipar].data_at_exec = FALSE;
>
> This code tries to dereferenciate the (still uninitialised) pointer I
> pass it ( that &(res->value_ind) ) and fails. The test should be like
> this instead:
>
> /* Data at exec macro only valid for C char/binary data */
> if (pcbValue && (pcbValue == SQL_DATA_AT_EXEC ||
> pcbValue <=
> SQL_LEN_DATA_AT_EXEC_OFFSET)) apdopts->parameters[ipar].data_at_exec =
> TRUE;
> else
> apdopts->parameters[ipar].data_at_exec = FALSE;
>
> ODBC has this dirty habit of passing negative integers as special
> values for pointers, and SQL_DATA_AT_EXEC is one of those:
>
> from /usr/include/sql.h:29:
>
> #define SQL_NULL_DATA (-1)
> #define SQL_DATA_AT_EXEC (-2)
>
> same goes for the handling of SQL_LEN_DATA_AT_EXEC_OFFSET.
>
> I regret I can't test the patch with code that uses SQL_DATA_AT_EXEC,
> because that is an ODBC feature that I do not use. However, with the
> patch my SQLBindParameter above works fine.
>
>
> Ciao,
>
> Enrico

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

Re: Fwd: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC breaks passing char buffer length

am 08.11.2006 17:03:01 von Peter Eisentraut

Am Dienstag, 7. November 2006 02:10 schrieb Hiroshi Inoue:
> Enrico is right at the point that the evaluation of *pcbValue here is
> dangerous and has no meaning. So in my version I already invalidated
> the code(#ifdef NOT_USED). Unfortunately his patch seems to have no
> meaning.

Which version is your version?

--
Peter Eisentraut
http://developer.postgresql.org/~petere/

---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

Re: Fwd: Bug#396218: bind.c: bad handling of SQL_LEN_DATA_AT_EXEC

am 08.11.2006 22:05:50 von Hiroshi Inoue

Peter Eisentraut wrote:
> Am Dienstag, 7. November 2006 02:10 schrieb Hiroshi Inoue:
>
>> Enrico is right at the point that the evaluation of *pcbValue here is
>> dangerous and has no meaning. So in my version I already invalidated
>> the code(#ifdef NOT_USED). Unfortunately his patch seems to have no
>> meaning.
>>
>
> Which version is your version?
>

The current 8.2.xxxx version which is basically different from the
8.1.xxxx version.

regards,
Hiroshi Inoue


---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend