public inbox for [email protected]
help / color / mirror / Atom feedpsqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
6+ messages / 2 participants
[nested] [flat]
* psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
@ 2024-02-22 06:29 Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Sukhanov @ 2024-02-22 06:29 UTC (permalink / raw)
To: [email protected]
Dear pgsql-odbc developers,
Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
Getting certain amount of diagnostic records with SQLGetDiagRecW crashes
the application with memory access violation.
Steps to reproduce:
1. Create procedure:
CREATE OR REPLACE PROCEDURE crashme()
LANGUAGE plpgsql
AS $$
BEGIN
FOR i IN 1..841 LOOP
RAISE NOTICE 'msgmsgmsgmsg (%)', i;
END LOOP;
END; $$;
2. Use example code in attachments.
3. Application crashes with memory access violation after calling
SQLGetDiagRecW function, with iRecord = 332.
Application doesn't crash if number of iterations in procedure's for
loop is changed.
Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA when there's
no more diagnostic records.
Regards,
Andrey
Attachments:
[text/x-c++src] sqlgetdiagrec_crash.cpp (3.4K, 2-sqlgetdiagrec_crash.cpp)
download | inline:
#include <iostream>
#include <memory>
#include <string>
#include <sql.h>
#include <sqlext.h>
// Collects diagnostic records without without saving until SQL_NO_DATA.
void CollectDiagRecords(SQLSMALLINT hndlType, SQLHANDLE hndl)
{
constexpr size_t msgBufferSize = 100;
SQLINTEGER error;
SQLSMALLINT msgSize{ 0 };
SQLWCHAR msgBuffer[msgBufferSize];
SQLWCHAR state[SQL_SQLSTATE_SIZE + 1];
for(SQLSMALLINT index = 1; ; ++index)
{
// Crashes after calling this function with index == 332. It is dependant on msgBufferSize and SQL procedure.
auto diagRet = SQLGetDiagRec(
hndlType,
hndl,
index,
state,
&error,
msgBuffer,
(SQLSMALLINT)msgBufferSize,
&msgSize);
switch(diagRet)
{
case SQL_SUCCESS:
//BOOST_LOG_TRIVIAL(info) << index << " " << boost::locale::conv::utf_to_utf<char>(state, state + SQL_SQLSTATE_SIZE) << " " << boost::locale::conv::utf_to_utf<char>(msgBuffer, msgBuffer + msgSize) << std::endl;
continue;
case SQL_SUCCESS_WITH_INFO:
{
auto bufferSize = msgSize + 1;
std::unique_ptr< SQLWCHAR[] > buf{ new SQLWCHAR[bufferSize] };
SQLGetDiagRec(hndlType, hndl, index, state, &error, buf.get(), bufferSize, &msgSize);
}
continue;
case SQL_INVALID_HANDLE:
throw std::runtime_error("CollectDiagRecords SQL_INVALID_HANDLE");
case SQL_ERROR:
throw std::runtime_error("CollectDiagRecords SQL_ERROR");
case SQL_NO_DATA:
// End collecting diag info
return;
}
}
}
int main(int argc, char** argv)
{
try
{
SQLHDBC hdbc = nullptr;
SQLHENV henv = nullptr;
// Allocate environment handle
auto ret = SQLAllocHandle(SQL_HANDLE_ENV, SQL_NULL_HANDLE, &henv);
if(ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO)
{
throw std::runtime_error("SQLAllocHandle SQL_HANDLE_ENV");
}
SQLSetEnvAttr(henv, SQL_ATTR_ODBC_VERSION, (void*)SQL_OV_ODBC3, 0);
// Allocate connection handle
ret = SQLAllocHandle(SQL_HANDLE_DBC, henv, &hdbc);
if(ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO)
{
CollectDiagRecords(SQL_HANDLE_ENV, henv);
throw std::runtime_error("SQLAllocHandle SQL_HANDLE_DBC");
}
// Connect
SQLWCHAR connString[] = L"Driver={PostgreSQL Unicode(x64)};Server=127.0.0.1;Port=5433;Database=crashdb;Uid=user1;Pwd=user1;";
ret = SQLDriverConnect(hdbc, nullptr, connString, sizeof(connString)/sizeof(SQLWCHAR), nullptr, 0, nullptr, SQL_DRIVER_NOPROMPT);
if(ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO)
{
CollectDiagRecords(SQL_HANDLE_DBC, hdbc);
throw std::runtime_error("SQLDriverConnect");
}
SQLHSTMT hstmt;
ret = SQLAllocHandle(SQL_HANDLE_STMT, hdbc, &hstmt);
if(ret != SQL_SUCCESS && ret != SQL_SUCCESS_WITH_INFO)
{
throw std::runtime_error("SQLAllocHandle SQL_HANDLE_STMT");
}
SQLWCHAR query[] = L"CALL public.crashme()";
ret = SQLExecDirect(hstmt, query, sizeof(query)/sizeof(SQLWCHAR));
switch(ret)
{
case SQL_SUCCESS:
break;
case SQL_SUCCESS_WITH_INFO:
// SQL Procedure crashme() returns "NOTICE" messages, which causes SQLExecDirect return SQL_SUCCESS_WITH_INFO.
CollectDiagRecords(SQL_HANDLE_STMT, hstmt);
break;
default:
throw std::runtime_error("SQLExecDirect default");
}
SQLFreeHandle(SQL_HANDLE_STMT, hstmt);
SQLDisconnect(hdbc);
SQLFreeHandle(SQL_HANDLE_DBC, hdbc);
SQLFreeHandle(SQL_HANDLE_ENV, henv);
}
catch(std::exception& e)
{
std::cerr << e.what() << std::endl;
return -1;
}
return 0;
}
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
@ 2024-02-26 20:11 ` Dave Cramer <[email protected]>
2024-02-27 06:07 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Dave Cramer @ 2024-02-26 20:11 UTC (permalink / raw)
To: Andrey Sukhanov <[email protected]>; +Cc: [email protected]
Hi Andrey,
I've tried on PostgreSQL 11 and 15 with no luck to replicate this problem.
Is it possible for you to debug it and send more information?
Dave Cramer
www.postgres.rocks
On Thu, 22 Feb 2024 at 01:29, Andrey Sukhanov <[email protected]> wrote:
> Dear pgsql-odbc developers,
>
> Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
> Getting certain amount of diagnostic records with SQLGetDiagRecW crashes
> the application with memory access violation.
>
> Steps to reproduce:
> 1. Create procedure:
> CREATE OR REPLACE PROCEDURE crashme()
> LANGUAGE plpgsql
> AS $$
> BEGIN
> FOR i IN 1..841 LOOP
> RAISE NOTICE 'msgmsgmsgmsg (%)', i;
> END LOOP;
> END; $$;
>
> 2. Use example code in attachments.
> 3. Application crashes with memory access violation after calling
> SQLGetDiagRecW function, with iRecord = 332.
> Application doesn't crash if number of iterations in procedure's for
> loop is changed.
> Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA when there's
> no more diagnostic records.
>
> Regards,
> Andrey
>
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
@ 2024-02-27 06:07 ` Andrey Sukhanov <[email protected]>
2024-02-27 14:21 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Sukhanov @ 2024-02-27 06:07 UTC (permalink / raw)
To: Dave Cramer <[email protected]>; +Cc: [email protected]
Hi Dave,
Thank you for your time.
The problem is caused by signed integer overflow, which leads to attempt
to read from unallocated memory.
I have downloaded psqlodbc 16 and debug symbols from
https://www.postgresql.org/ftp/odbc/versions/.
Source code from for psqlodbc 16 from
https://git.postgresql.org/git/psqlodbc.git.
I compiled the attached program with msvc 14 and msvc 16 compilers (same
results). Program should be compiled with defined "UNICODE" to use
functions (SQLGetDiagRecW etc.) that accept wchar.
On windows 10 attached program crashes inside ER_ReturnError line
environ.c:249. Where it tries to memcpy from the buffer "msg" with
offset "stapos".
"msg" is a valid pointer to "pgerror->__error_msg". "pgerror" also have
field "pgerror->errorsize" with value "32690".
Offset "stapos" is calculated as "stapos = (RecNumber - 1) *
error->recsize;"
Here "error" is "pgerror", "recsize" = 99, "RecNumber" = 332.
"stapos" should be assigned the value of 32769 but it has type SWORD
(short int) (INT16_MAX is 32767).
Expression"if (stapos > msglen)" (here "msglen" = 32690) evaluates to
"false", because of signed integer overflow. It should've been "true"
instead, and function would've returned SQL_NO_DATA_FOUND.
Callstack:
vcruntime140.dll!memcpy() line 438
psqlodbc35w.dll!ER_ReturnError(PG_ErrorInfo * pgerror, short
RecNumber, unsigned char * szSqlState, long * pfNativeError, unsigned
char * szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned
short) line 250
psqlodbc35w.dll!PGAPI_StmtError(void * hstmt, short RecNumber,
unsigned char * szSqlState, long * pfNativeError, unsigned char *
szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned short)
line 1626
psqlodbc35w.dll!PGAPI_GetDiagRec(short) line 57
psqlodbc35w.dll!SQLGetDiagRecW(short fHandleType, void * handle,
short iRecord, wchar_t * szSqlState, long * pfNativeError, wchar_t *
szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg) line 234
odbc32.dll!SQLGetDiagRecW()
odbc.pg.exe!CollectDiagRecords(short hndlType, void * hndl) line 30
odbc.pg.exe!main(int argc, char * * argv) line 117
That means, that the problem is caused by signed integer overflow, which
in turn causes memcpy to read from unallocated memory.
The easiest way to reproduce this problem is to try increasing the
number of "raise notice" messages in for loop. If their number gets past
certain point, no diagnostic records from SQLGetDiagRec are being
returned at all. Then I've decreased the number of these messages until
they started appearing again. In my case at this number I am able to
reproduce the problem. With further decrease of this number problem
doesn't appear anymore.
I can't fix this problem myself, because I'm not familiar with the
project, and I don't know how to properly test psqlodbc on different
platforms.
Regards,
Andrey
27.02.2024 3:11, Dave Cramer wrote:
> Hi Andrey,
>
> I've tried on PostgreSQL 11 and 15 with no luck to replicate this problem.
>
> Is it possible for you to debug it and send more information?
>
> Dave Cramer
> www.postgres.rocks
>
>
> On Thu, 22 Feb 2024 at 01:29, Andrey Sukhanov <[email protected]> wrote:
>
> Dear pgsql-odbc developers,
>
> Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
> Getting certain amount of diagnostic records with SQLGetDiagRecW
> crashes
> the application with memory access violation.
>
> Steps to reproduce:
> 1. Create procedure:
> CREATE OR REPLACE PROCEDURE crashme()
> LANGUAGE plpgsql
> AS $$
> BEGIN
> FOR i IN 1..841 LOOP
> RAISE NOTICE 'msgmsgmsgmsg (%)', i;
> END LOOP;
> END; $$;
>
> 2. Use example code in attachments.
> 3. Application crashes with memory access violation after calling
> SQLGetDiagRecW function, with iRecord = 332.
> Application doesn't crash if number of iterations in procedure's for
> loop is changed.
> Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA when
> there's
> no more diagnostic records.
>
> Regards,
> Andrey
>
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
2024-02-27 06:07 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
@ 2024-02-27 14:21 ` Dave Cramer <[email protected]>
2024-02-28 04:33 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Dave Cramer @ 2024-02-27 14:21 UTC (permalink / raw)
To: Andrey Sukhanov <[email protected]>; +Cc: [email protected]
Hi Andrey,
Thoughts use unsigned word for lengths to avoid overflow by davecramer ·
Pull Request #3 · postgresql-interfaces/psqlodbc (github.com)
<https://github.com/postgresql-interfaces/psqlodbc/pull/3;
Dave Cramer
www.postgres.rocks
On Tue, 27 Feb 2024 at 01:07, Andrey Sukhanov <[email protected]> wrote:
> Hi Dave,
>
> Thank you for your time.
> The problem is caused by signed integer overflow, which leads to attempt
> to read from unallocated memory.
>
> I have downloaded psqlodbc 16 and debug symbols from
> https://www.postgresql.org/ftp/odbc/versions/.
> Source code from for psqlodbc 16 from
> https://git.postgresql.org/git/psqlodbc.git.
> I compiled the attached program with msvc 14 and msvc 16 compilers (same
> results). Program should be compiled with defined "UNICODE" to use
> functions (SQLGetDiagRecW etc.) that accept wchar.
>
> On windows 10 attached program crashes inside ER_ReturnError line
> environ.c:249. Where it tries to memcpy from the buffer "msg" with offset
> "stapos".
>
>
> "msg" is a valid pointer to "pgerror->__error_msg". "pgerror" also have
> field "pgerror->errorsize" with value "32690".
> Offset "stapos" is calculated as "stapos = (RecNumber - 1) *
> error->recsize;"
> Here "error" is "pgerror", "recsize" = 99, "RecNumber" = 332.
> "stapos" should be assigned the value of 32769 but it has type SWORD
> (short int) (INT16_MAX is 32767).
> Expression"if (stapos > msglen)" (here "msglen" = 32690) evaluates to
> "false", because of signed integer overflow. It should've been "true"
> instead, and function would've returned SQL_NO_DATA_FOUND.
> Callstack:
> vcruntime140.dll!memcpy() line 438
> psqlodbc35w.dll!ER_ReturnError(PG_ErrorInfo * pgerror, short
> RecNumber, unsigned char * szSqlState, long * pfNativeError, unsigned char
> * szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned short)
> line 250
> psqlodbc35w.dll!PGAPI_StmtError(void * hstmt, short RecNumber,
> unsigned char * szSqlState, long * pfNativeError, unsigned char *
> szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned short) line
> 1626
> psqlodbc35w.dll!PGAPI_GetDiagRec(short) line 57
> psqlodbc35w.dll!SQLGetDiagRecW(short fHandleType, void * handle,
> short iRecord, wchar_t * szSqlState, long * pfNativeError, wchar_t *
> szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg) line 234
> odbc32.dll!SQLGetDiagRecW()
> odbc.pg.exe!CollectDiagRecords(short hndlType, void * hndl) line 30
> odbc.pg.exe!main(int argc, char * * argv) line 117
>
> That means, that the problem is caused by signed integer overflow, which
> in turn causes memcpy to read from unallocated memory.
> The easiest way to reproduce this problem is to try increasing the number
> of "raise notice" messages in for loop. If their number gets past certain
> point, no diagnostic records from SQLGetDiagRec are being returned at all.
> Then I've decreased the number of these messages until they started
> appearing again. In my case at this number I am able to reproduce the
> problem. With further decrease of this number problem doesn't appear
> anymore.
>
> I can't fix this problem myself, because I'm not familiar with the
> project, and I don't know how to properly test psqlodbc on different
> platforms.
>
> Regards,
> Andrey
>
>
> 27.02.2024 3:11, Dave Cramer wrote:
>
> Hi Andrey,
>
> I've tried on PostgreSQL 11 and 15 with no luck to replicate this problem.
>
> Is it possible for you to debug it and send more information?
>
> Dave Cramer
> www.postgres.rocks
>
>
> On Thu, 22 Feb 2024 at 01:29, Andrey Sukhanov <[email protected]> wrote:
>
>> Dear pgsql-odbc developers,
>>
>> Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
>> Getting certain amount of diagnostic records with SQLGetDiagRecW crashes
>> the application with memory access violation.
>>
>> Steps to reproduce:
>> 1. Create procedure:
>> CREATE OR REPLACE PROCEDURE crashme()
>> LANGUAGE plpgsql
>> AS $$
>> BEGIN
>> FOR i IN 1..841 LOOP
>> RAISE NOTICE 'msgmsgmsgmsg (%)', i;
>> END LOOP;
>> END; $$;
>>
>> 2. Use example code in attachments.
>> 3. Application crashes with memory access violation after calling
>> SQLGetDiagRecW function, with iRecord = 332.
>> Application doesn't crash if number of iterations in procedure's for
>> loop is changed.
>> Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA when there's
>> no more diagnostic records.
>>
>> Regards,
>> Andrey
>>
>
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
2024-02-27 06:07 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-27 14:21 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
@ 2024-02-28 04:33 ` Andrey Sukhanov <[email protected]>
2024-02-28 13:21 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Andrey Sukhanov @ 2024-02-28 04:33 UTC (permalink / raw)
To: Dave Cramer <[email protected]>; +Cc: [email protected]
Hi Dave,
I think it should solve this problem. Thank you for your help.
It seems that there won't be a problem with use of unsigned values,
since all values that could be negative are handled.
I'm not sure if "stapos" should be short. Product of 2byte*2byte
variables could require 4 bytes to hold the value,
if it's possible for "error->recsize" and "RecNumber" to hold big enough
values at the same time.
Maybe make "stapos" signed/unsigned int to make sure it won't happen?
Regards,
Andrey
27.02.2024 21:21, Dave Cramer wrote:
> Hi Andrey,
>
> Thoughts use unsigned word for lengths to avoid overflow by davecramer
> · Pull Request #3 · postgresql-interfaces/psqlodbc (github.com)
> <https://github.com/postgresql-interfaces/psqlodbc/pull/3;
>
> Dave Cramer
> www.postgres.rocks
>
>
> On Tue, 27 Feb 2024 at 01:07, Andrey Sukhanov <[email protected]> wrote:
>
> Hi Dave,
>
> Thank you for your time.
> The problem is caused by signed integer overflow, which leads to
> attempt to read from unallocated memory.
>
> I have downloaded psqlodbc 16 and debug symbols from
> https://www.postgresql.org/ftp/odbc/versions/.
> Source code from for psqlodbc 16 from
> https://git.postgresql.org/git/psqlodbc.git.
> I compiled the attached program with msvc 14 and msvc 16 compilers
> (same results). Program should be compiled with defined "UNICODE"
> to use functions (SQLGetDiagRecW etc.) that accept wchar.
>
> On windows 10 attached program crashes inside ER_ReturnError line
> environ.c:249. Where it tries to memcpy from the buffer "msg" with
> offset "stapos".
>
>
> "msg" is a valid pointer to "pgerror->__error_msg". "pgerror" also
> have field "pgerror->errorsize" with value "32690".
> Offset "stapos" is calculated as "stapos = (RecNumber - 1) *
> error->recsize;"
> Here "error" is "pgerror", "recsize" = 99, "RecNumber" = 332.
> "stapos" should be assigned the value of 32769 but it has type
> SWORD (short int) (INT16_MAX is 32767).
> Expression"if (stapos > msglen)" (here "msglen" = 32690) evaluates
> to "false", because of signed integer overflow. It should've been
> "true" instead, and function would've returned SQL_NO_DATA_FOUND.
> Callstack:
> vcruntime140.dll!memcpy() line 438
> psqlodbc35w.dll!ER_ReturnError(PG_ErrorInfo * pgerror, short
> RecNumber, unsigned char * szSqlState, long * pfNativeError,
> unsigned char * szErrorMsg, short cbErrorMsgMax, short *
> pcbErrorMsg, unsigned short) line 250
> psqlodbc35w.dll!PGAPI_StmtError(void * hstmt, short
> RecNumber, unsigned char * szSqlState, long * pfNativeError,
> unsigned char * szErrorMsg, short cbErrorMsgMax, short *
> pcbErrorMsg, unsigned short) line 1626
> psqlodbc35w.dll!PGAPI_GetDiagRec(short) line 57
> psqlodbc35w.dll!SQLGetDiagRecW(short fHandleType, void *
> handle, short iRecord, wchar_t * szSqlState, long * pfNativeError,
> wchar_t * szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg)
> line 234
> odbc32.dll!SQLGetDiagRecW()
> odbc.pg.exe!CollectDiagRecords(short hndlType, void * hndl)
> line 30
> odbc.pg.exe!main(int argc, char * * argv) line 117
>
> That means, that the problem is caused by signed integer overflow,
> which in turn causes memcpy to read from unallocated memory.
> The easiest way to reproduce this problem is to try increasing the
> number of "raise notice" messages in for loop. If their number
> gets past certain point, no diagnostic records from SQLGetDiagRec
> are being returned at all. Then I've decreased the number of these
> messages until they started appearing again. In my case at this
> number I am able to reproduce the problem. With further decrease
> of this number problem doesn't appear anymore.
>
> I can't fix this problem myself, because I'm not familiar with the
> project, and I don't know how to properly test psqlodbc on
> different platforms.
>
> Regards,
> Andrey
>
>
> 27.02.2024 3:11, Dave Cramer wrote:
>> Hi Andrey,
>>
>> I've tried on PostgreSQL 11 and 15 with no luck to replicate this
>> problem.
>>
>> Is it possible for you to debug it and send more information?
>>
>> Dave Cramer
>> www.postgres.rocks <http://www.postgres.rocks;
>>
>>
>> On Thu, 22 Feb 2024 at 01:29, Andrey Sukhanov
>> <[email protected]> wrote:
>>
>> Dear pgsql-odbc developers,
>>
>> Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
>> Getting certain amount of diagnostic records with
>> SQLGetDiagRecW crashes
>> the application with memory access violation.
>>
>> Steps to reproduce:
>> 1. Create procedure:
>> CREATE OR REPLACE PROCEDURE crashme()
>> LANGUAGE plpgsql
>> AS $$
>> BEGIN
>> FOR i IN 1..841 LOOP
>> RAISE NOTICE 'msgmsgmsgmsg (%)', i;
>> END LOOP;
>> END; $$;
>>
>> 2. Use example code in attachments.
>> 3. Application crashes with memory access violation after
>> calling
>> SQLGetDiagRecW function, with iRecord = 332.
>> Application doesn't crash if number of iterations in
>> procedure's for
>> loop is changed.
>> Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA
>> when there's
>> no more diagnostic records.
>>
>> Regards,
>> Andrey
>>
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
2024-02-27 06:07 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-27 14:21 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Dave Cramer <[email protected]>
2024-02-28 04:33 ` Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
@ 2024-02-28 13:21 ` Dave Cramer <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Dave Cramer @ 2024-02-28 13:21 UTC (permalink / raw)
To: Andrey Sukhanov <[email protected]>; +Cc: [email protected]
Hi Andrey,
Probably makes sense. Not sure why the original authors used such small ints
Dave Cramer
www.postgres.rocks
On Tue, 27 Feb 2024 at 23:33, Andrey Sukhanov <[email protected]> wrote:
> Hi Dave,
>
> I think it should solve this problem. Thank you for your help.
>
> It seems that there won't be a problem with use of unsigned values, since
> all values that could be negative are handled.
> I'm not sure if "stapos" should be short. Product of 2byte*2byte
> variables could require 4 bytes to hold the value,
> if it's possible for "error->recsize" and "RecNumber" to hold big enough
> values at the same time.
> Maybe make "stapos" signed/unsigned int to make sure it won't happen?
>
> Regards,
> Andrey
>
>
> 27.02.2024 21:21, Dave Cramer wrote:
>
> Hi Andrey,
>
> Thoughts use unsigned word for lengths to avoid overflow by davecramer ·
> Pull Request #3 · postgresql-interfaces/psqlodbc (github.com)
> <https://github.com/postgresql-interfaces/psqlodbc/pull/3;
>
> Dave Cramer
> www.postgres.rocks
>
>
> On Tue, 27 Feb 2024 at 01:07, Andrey Sukhanov <[email protected]> wrote:
>
>> Hi Dave,
>>
>> Thank you for your time.
>> The problem is caused by signed integer overflow, which leads to attempt
>> to read from unallocated memory.
>>
>> I have downloaded psqlodbc 16 and debug symbols from
>> https://www.postgresql.org/ftp/odbc/versions/.
>> Source code from for psqlodbc 16 from
>> https://git.postgresql.org/git/psqlodbc.git.
>> I compiled the attached program with msvc 14 and msvc 16 compilers (same
>> results). Program should be compiled with defined "UNICODE" to use
>> functions (SQLGetDiagRecW etc.) that accept wchar.
>>
>> On windows 10 attached program crashes inside ER_ReturnError line
>> environ.c:249. Where it tries to memcpy from the buffer "msg" with offset
>> "stapos".
>>
>>
>> "msg" is a valid pointer to "pgerror->__error_msg". "pgerror" also have
>> field "pgerror->errorsize" with value "32690".
>> Offset "stapos" is calculated as "stapos = (RecNumber - 1) *
>> error->recsize;"
>> Here "error" is "pgerror", "recsize" = 99, "RecNumber" = 332.
>> "stapos" should be assigned the value of 32769 but it has type SWORD
>> (short int) (INT16_MAX is 32767).
>> Expression"if (stapos > msglen)" (here "msglen" = 32690) evaluates to
>> "false", because of signed integer overflow. It should've been "true"
>> instead, and function would've returned SQL_NO_DATA_FOUND.
>> Callstack:
>> vcruntime140.dll!memcpy() line 438
>> psqlodbc35w.dll!ER_ReturnError(PG_ErrorInfo * pgerror, short
>> RecNumber, unsigned char * szSqlState, long * pfNativeError, unsigned char
>> * szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned short)
>> line 250
>> psqlodbc35w.dll!PGAPI_StmtError(void * hstmt, short RecNumber,
>> unsigned char * szSqlState, long * pfNativeError, unsigned char *
>> szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg, unsigned short) line
>> 1626
>> psqlodbc35w.dll!PGAPI_GetDiagRec(short) line 57
>> psqlodbc35w.dll!SQLGetDiagRecW(short fHandleType, void * handle,
>> short iRecord, wchar_t * szSqlState, long * pfNativeError, wchar_t *
>> szErrorMsg, short cbErrorMsgMax, short * pcbErrorMsg) line 234
>> odbc32.dll!SQLGetDiagRecW()
>> odbc.pg.exe!CollectDiagRecords(short hndlType, void * hndl) line 30
>> odbc.pg.exe!main(int argc, char * * argv) line 117
>>
>> That means, that the problem is caused by signed integer overflow, which
>> in turn causes memcpy to read from unallocated memory.
>> The easiest way to reproduce this problem is to try increasing the number
>> of "raise notice" messages in for loop. If their number gets past certain
>> point, no diagnostic records from SQLGetDiagRec are being returned at all.
>> Then I've decreased the number of these messages until they started
>> appearing again. In my case at this number I am able to reproduce the
>> problem. With further decrease of this number problem doesn't appear
>> anymore.
>>
>> I can't fix this problem myself, because I'm not familiar with the
>> project, and I don't know how to properly test psqlodbc on different
>> platforms.
>>
>> Regards,
>> Andrey
>>
>>
>> 27.02.2024 3:11, Dave Cramer wrote:
>>
>> Hi Andrey,
>>
>> I've tried on PostgreSQL 11 and 15 with no luck to replicate this problem.
>>
>> Is it possible for you to debug it and send more information?
>>
>> Dave Cramer
>> www.postgres.rocks
>>
>>
>> On Thu, 22 Feb 2024 at 01:29, Andrey Sukhanov <[email protected]> wrote:
>>
>>> Dear pgsql-odbc developers,
>>>
>>> Windows 10, psqlodbc 16 (psqlodbc35w.dll), postgresql 11.
>>> Getting certain amount of diagnostic records with SQLGetDiagRecW crashes
>>> the application with memory access violation.
>>>
>>> Steps to reproduce:
>>> 1. Create procedure:
>>> CREATE OR REPLACE PROCEDURE crashme()
>>> LANGUAGE plpgsql
>>> AS $$
>>> BEGIN
>>> FOR i IN 1..841 LOOP
>>> RAISE NOTICE 'msgmsgmsgmsg (%)', i;
>>> END LOOP;
>>> END; $$;
>>>
>>> 2. Use example code in attachments.
>>> 3. Application crashes with memory access violation after calling
>>> SQLGetDiagRecW function, with iRecord = 332.
>>> Application doesn't crash if number of iterations in procedure's for
>>> loop is changed.
>>> Expected outcome: SQLGetDiagRecW would return SQL_NO_DATA when there's
>>> no more diagnostic records.
>>>
>>> Regards,
>>> Andrey
>>>
>>
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-02-28 13:21 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-22 06:29 psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW Andrey Sukhanov <[email protected]>
2024-02-26 20:11 ` Dave Cramer <[email protected]>
2024-02-27 06:07 ` Andrey Sukhanov <[email protected]>
2024-02-27 14:21 ` Dave Cramer <[email protected]>
2024-02-28 04:33 ` Andrey Sukhanov <[email protected]>
2024-02-28 13:21 ` Dave Cramer <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox