public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andrey Sukhanov <[email protected]>
To: Dave Cramer <[email protected]>
Cc: [email protected]
Subject: Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
Date: Wed, 28 Feb 2024 11:33:31 +0700
Message-ID: <[email protected]> (raw)
In-Reply-To: <CADK3HH+w+W=750pp3XwpwCiHrQCvOYhLyFZyp8EirirVLHGPrQ@mail.gmail.com>
References: <[email protected]>
<CADK3HHJLym2DzrBkrUbLysDE9K=-73g8XtxFpMKgdaWho0Jdew@mail.gmail.com>
<[email protected]>
<CADK3HH+w+W=750pp3XwpwCiHrQCvOYhLyFZyp8EirirVLHGPrQ@mail.gmail.com>
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
>>
view thread (6+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: psqlodbc crashes while collecting diagnostic records with SQLGetDiagRecW
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox