Buffer overflow in extract_diag_error_w()

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Buffer overflow in extract_diag_error_w()

Andrew Punch
Hi,

I am working with Netezza in Perl. The script was crashing with a
segfault. When I used gdb to investigate the problem was isolated to the
wide_strcpy() call in extract_diag_error_w() due to a larger than
expected error message from the Netezza driver. The error message from
the Netezza driver was not corrupt and was in the correct format.

I solved this problem in the short term by increasing
SQL_MAX_MESSAGE_LENGTH from 512 to 1024. Ideally something like
wcscpy_s() should be used.

Thanks
-Andrew

diff -ur unixODBC-2.3.1/include/sql.h unixodbc-2.3.1-patched/include/sql.h
--- unixODBC-2.3.1/include/sql.h    2011-07-04 18:54:09.000000000 +1000
+++ unixodbc-2.3.1-patched/include/sql.h    2015-11-11
16:13:26.709201111 +1100
@@ -46,7 +46,7 @@
  #define SQL_NTSL                  (-3L)

  /* maximum message length */
-#define SQL_MAX_MESSAGE_LENGTH   512
+#define SQL_MAX_MESSAGE_LENGTH   1024

  /* date/time length constants */
  #if (ODBCVER >= 0x0300)
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overflow in extract_diag_error_w()

Nick Gorham-2
On 11/11/15 06:32, Andrew Punch wrote:

> Hi,
>
> I am working with Netezza in Perl. The script was crashing with a
> segfault. When I used gdb to investigate the problem was isolated to
> the wide_strcpy() call in extract_diag_error_w() due to a larger than
> expected error message from the Netezza driver. The error message from
> the Netezza driver was not corrupt and was in the correct format.
>
> I solved this problem in the short term by increasing
> SQL_MAX_MESSAGE_LENGTH from 512 to 1024. Ideally something like
> wcscpy_s() should be used.
>
> Thanks
> -Andrew
>
> diff -ur unixODBC-2.3.1/include/sql.h
> unixodbc-2.3.1-patched/include/sql.h
> --- unixODBC-2.3.1/include/sql.h    2011-07-04 18:54:09.000000000 +1000
> +++ unixodbc-2.3.1-patched/include/sql.h    2015-11-11
> 16:13:26.709201111 +1100
> @@ -46,7 +46,7 @@
>  #define SQL_NTSL                  (-3L)
>
>  /* maximum message length */
> -#define SQL_MAX_MESSAGE_LENGTH   512
> +#define SQL_MAX_MESSAGE_LENGTH   1024
>
>  /* date/time length constants */
>  #if (ODBCVER >= 0x0300)
> _______________________________________________
> unixODBC-dev mailing list
> [hidden email]
> http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev

Hi, SQL_MAX_MESSAGE_LENGTH is defined by the MS headers so we don't want
to alter that, but have you tried a current build of unixODBC, as 2.3.1
was from 26th November 2011. I think the problem is fixed in 2.3.3, the
changes contain "Fix buffer overrun returning long diagnostic from driver".

Try the current, 2.3.5-pre and if the problem is still there, provide
details and we will sort it out. How long is the message that comes from
the driver?

--
Nick




_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overflow in extract_diag_error_w()

Andrew Punch
In reply to this post by Andrew Punch
Hi,

I rewrote the patch so that the message is truncated.

diff -ur unixODBC-2.3.1/DriverManager/__info.c
unixodbc-2.3.1-patched/DriverManager/__info.c
--- unixODBC-2.3.1/DriverManager/__info.c   2011-11-15
22:43:15.000000000 +1100
+++ unixodbc-2.3.1-patched/DriverManager/__info.c   2015-11-11
20:16:24.585057248 +1100
@@ -4460,7 +4460,7 @@
              SQLWCHAR *tmp;

  #ifdef STRICT_ODBC_ERROR
-            wide_strcpy( msg, msg1 );
+            wide_strncpy( msg, msg1, SQL_MAX_MESSAGE_LENGTH);
  #else
              tmp = ansi_to_unicode_alloc((SQLCHAR*) ERROR_PREFIX,
SQL_NTS, connection );
              wide_strcpy( msg, tmp );
@@ -4640,7 +4640,7 @@
                  as1 = (SQLCHAR*) unicode_to_ansi_alloc( sqlstate,
SQL_NTS, connection );
                  as2 = (SQLCHAR*) unicode_to_ansi_alloc( msg1, SQL_NTS,
connection );

-                sprintf( connection -> msg, "\t\tDIAG [%s] %s",
+                snprintf( connection -> msg, SQL_MAX_MESSAGE_LENGTH,
"\t\tDIAG [%s] %s",
                          as1, as2 );

                  if( as1 ) free( as1 );
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overflow in extract_diag_error_w()

Nick Gorham-2
On 11/11/15 10:22, Andrew Punch wrote:

> Hi,
>
> I rewrote the patch so that the message is truncated.
>
> diff -ur unixODBC-2.3.1/DriverManager/__info.c
> unixodbc-2.3.1-patched/DriverManager/__info.c
> --- unixODBC-2.3.1/DriverManager/__info.c   2011-11-15
> 22:43:15.000000000 +1100
> +++ unixodbc-2.3.1-patched/DriverManager/__info.c   2015-11-11
> 20:16:24.585057248 +1100
> @@ -4460,7 +4460,7 @@
>              SQLWCHAR *tmp;
>
>  #ifdef STRICT_ODBC_ERROR
> -            wide_strcpy( msg, msg1 );
> +            wide_strncpy( msg, msg1, SQL_MAX_MESSAGE_LENGTH);
>  #else
>              tmp = ansi_to_unicode_alloc((SQLCHAR*) ERROR_PREFIX,
> SQL_NTS, connection );
>              wide_strcpy( msg, tmp );
> @@ -4640,7 +4640,7 @@
>                  as1 = (SQLCHAR*) unicode_to_ansi_alloc( sqlstate,
> SQL_NTS, connection );
>                  as2 = (SQLCHAR*) unicode_to_ansi_alloc( msg1,
> SQL_NTS, connection );
>
> -                sprintf( connection -> msg, "\t\tDIAG [%s] %s",
> +                snprintf( connection -> msg, SQL_MAX_MESSAGE_LENGTH,
> "\t\tDIAG [%s] %s",
>                          as1, as2 );
>
>                  if( as1 ) free( as1 );
> _______________________________________________
> unixODBC-dev mailing list
> [hidden email]
> http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev

Ok, but how does a message string thats longer than
SQL_MAX_MESSAGE_LENGTH come out of

         ret = SQLError(
                 henv,
                 hdbc,
                 hstmt,
                 sqlstate,
                 &native,
                 msg1,
                 sizeof( msg1 ),
                 &len );

Given that sizeof( msg1 ) == SQL_MAX_MESSAGE_LENGTH

Or an I still looking at different code to you in 2.3.1 and 2.3.5-pre?

--
Nick
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overflow in extract_diag_error_w()

Nick Gorham-2
On 11/11/15 10:36, Nick Gorham wrote:

> On 11/11/15 10:22, Andrew Punch wrote:
>> Hi,
>>
>> I rewrote the patch so that the message is truncated.
>>
>> diff -ur unixODBC-2.3.1/DriverManager/__info.c
>> unixodbc-2.3.1-patched/DriverManager/__info.c
>> --- unixODBC-2.3.1/DriverManager/__info.c   2011-11-15
>> 22:43:15.000000000 +1100
>> +++ unixodbc-2.3.1-patched/DriverManager/__info.c   2015-11-11
>> 20:16:24.585057248 +1100
>> @@ -4460,7 +4460,7 @@
>>              SQLWCHAR *tmp;
>>
>>  #ifdef STRICT_ODBC_ERROR
>> -            wide_strcpy( msg, msg1 );
>> +            wide_strncpy( msg, msg1, SQL_MAX_MESSAGE_LENGTH);
>>  #else
>>              tmp = ansi_to_unicode_alloc((SQLCHAR*) ERROR_PREFIX,
>> SQL_NTS, connection );
>>              wide_strcpy( msg, tmp );
>> @@ -4640,7 +4640,7 @@
>>                  as1 = (SQLCHAR*) unicode_to_ansi_alloc( sqlstate,
>> SQL_NTS, connection );
>>                  as2 = (SQLCHAR*) unicode_to_ansi_alloc( msg1,
>> SQL_NTS, connection );
>>
>> -                sprintf( connection -> msg, "\t\tDIAG [%s] %s",
>> +                snprintf( connection -> msg, SQL_MAX_MESSAGE_LENGTH,
>> "\t\tDIAG [%s] %s",
>>                          as1, as2 );
>>
>>                  if( as1 ) free( as1 );
>> _______________________________________________
>> unixODBC-dev mailing list
>> [hidden email]
>> http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
>
> Ok, but how does a message string thats longer than
> SQL_MAX_MESSAGE_LENGTH come out of
>
>         ret = SQLError(
>                 henv,
>                 hdbc,
>                 hstmt,
>                 sqlstate,
>                 &native,
>                 msg1,
>                 sizeof( msg1 ),
>                 &len );
>
> Given that sizeof( msg1 ) == SQL_MAX_MESSAGE_LENGTH
>
> Or an I still looking at different code to you in 2.3.1 and 2.3.5-pre?
>

I have committed a change to 2.3.5-pre that should make sure that the
buffer is not overrun. But if the driver SQLError writes more that the
buffer length of characters to the supplied buffer thats out of my control.

--
Nick
_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
Reply | Threaded
Open this post in threaded view
|

Re: Buffer overflow in extract_diag_error_w()

Andrew Punch
In reply to this post by Andrew Punch
Hi,

I tested 2.3.4 and it does not segfault. The netezza driver is a bit
buggy, so I wouldn't be surprised if it is doing something wrong.

Thanks for your help.

-Andrew

On 11/11/15 23:00, [hidden email] wrote:

> Send unixODBC-dev mailing list submissions to
> [hidden email]
>
> To subscribe or unsubscribe via the World Wide Web, visit
> http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
> or, via email, send a message with subject or body 'help' to
> [hidden email]
>
> You can reach the person managing the list at
> [hidden email]
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of unixODBC-dev digest..."
>
>
> Today's Topics:
>
>     1. Buffer overflow in extract_diag_error_w() (Andrew Punch)
>     2. Re: Buffer overflow in extract_diag_error_w() (Nick Gorham)
>     3. Re: Buffer overflow in extract_diag_error_w() (Andrew Punch)
>     4. Re: Buffer overflow in extract_diag_error_w() (Nick Gorham)
>     5. Re: Buffer overflow in extract_diag_error_w() (Nick Gorham)
>
>
> ----------------------------------------------------------------------
>
> Message: 1
> Date: Wed, 11 Nov 2015 17:32:03 +1100
> From: Andrew Punch <[hidden email]>
> Subject: [unixODBC-dev] Buffer overflow in extract_diag_error_w()
> To: [hidden email]
> Message-ID: <[hidden email]>
> Content-Type: text/plain; charset=utf-8; format=flowed
>
> Hi,
>
> I am working with Netezza in Perl. The script was crashing with a
> segfault. When I used gdb to investigate the problem was isolated to the
> wide_strcpy() call in extract_diag_error_w() due to a larger than
> expected error message from the Netezza driver. The error message from
> the Netezza driver was not corrupt and was in the correct format.
>
> I solved this problem in the short term by increasing
> SQL_MAX_MESSAGE_LENGTH from 512 to 1024. Ideally something like
> wcscpy_s() should be used.
>
> Thanks
> -Andrew
>
> diff -ur unixODBC-2.3.1/include/sql.h unixodbc-2.3.1-patched/include/sql.h
> --- unixODBC-2.3.1/include/sql.h    2011-07-04 18:54:09.000000000 +1000
> +++ unixodbc-2.3.1-patched/include/sql.h    2015-11-11
> 16:13:26.709201111 +1100
> @@ -46,7 +46,7 @@
>    #define SQL_NTSL                  (-3L)
>
>    /* maximum message length */
> -#define SQL_MAX_MESSAGE_LENGTH   512
> +#define SQL_MAX_MESSAGE_LENGTH   1024
>
>    /* date/time length constants */
>    #if (ODBCVER >= 0x0300)
>
>
> ------------------------------
>
> Message: 2
> Date: Wed, 11 Nov 2015 09:50:42 +0000
> From: Nick Gorham <[hidden email]>
> Subject: Re: [unixODBC-dev] Buffer overflow in extract_diag_error_w()
> To: [hidden email]
> Message-ID: <[hidden email]>
> Content-Type: text/plain; charset=windows-1252; format=flowed
>
> On 11/11/15 06:32, Andrew Punch wrote:
>> Hi,
>>
>> I am working with Netezza in Perl. The script was crashing with a
>> segfault. When I used gdb to investigate the problem was isolated to
>> the wide_strcpy() call in extract_diag_error_w() due to a larger than
>> expected error message from the Netezza driver. The error message from
>> the Netezza driver was not corrupt and was in the correct format.
>>
>> I solved this problem in the short term by increasing
>> SQL_MAX_MESSAGE_LENGTH from 512 to 1024. Ideally something like
>> wcscpy_s() should be used.
>>
>> Thanks
>> -Andrew
>>
>> diff -ur unixODBC-2.3.1/include/sql.h
>> unixodbc-2.3.1-patched/include/sql.h
>> --- unixODBC-2.3.1/include/sql.h    2011-07-04 18:54:09.000000000 +1000
>> +++ unixodbc-2.3.1-patched/include/sql.h    2015-11-11
>> 16:13:26.709201111 +1100
>> @@ -46,7 +46,7 @@
>>   #define SQL_NTSL                  (-3L)
>>
>>   /* maximum message length */
>> -#define SQL_MAX_MESSAGE_LENGTH   512
>> +#define SQL_MAX_MESSAGE_LENGTH   1024
>>
>>   /* date/time length constants */
>>   #if (ODBCVER >= 0x0300)
>> _______________________________________________
>> unixODBC-dev mailing list
>> [hidden email]
>> http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev
> Hi, SQL_MAX_MESSAGE_LENGTH is defined by the MS headers so we don't want
> to alter that, but have you tried a current build of unixODBC, as 2.3.1
> was from 26th November 2011. I think the problem is fixed in 2.3.3, the
> changes contain "Fix buffer overrun returning long diagnostic from driver".
>
> Try the current, 2.3.5-pre and if the problem is still there, provide
> details and we will sort it out. How long is the message that comes from
> the driver?
>

_______________________________________________
unixODBC-dev mailing list
[hidden email]
http://mailman.unixodbc.org/mailman/listinfo/unixodbc-dev