Incorrect use of SQLColAttributes with ODBC 3.0

Bug #69354	Incorrect use of SQLColAttributes with ODBC 3.0
Submitted:	2015-04-02 08:35 UTC	Modified:	2015-04-06 15:23 UTC
From:	php at mdjnet dot dk	Assigned:	ab (profile)
Status:	Closed	Package:	ODBC related
PHP Version:	5.6.7	OS:	All
Private report:	No	CVE-ID:	None

[2015-04-02 08:35 UTC] php at mdjnet dot dk

Description:
------------
Since php was changed to odbc 3.0 (#68964), an old hidden bug has surfaced. This bug is related to both #68350 "SQL_DESC_OCTET_LENGTH not supported by ADS ODBC driver" and #68014 "Result data values can be truncated because of incorrect column display sizes".

In odbc 3.0, SQLColAttributes is deprecated, instead SQLColAttribute should be used, which in turn supports SQL_DESC_OCTET_LENGTH, introduced with odbc 3.0.

The bug is in ext/odbc/php_odbc.c, the function odbc_bindcols, in the middle of the big switch. With ODBCVER set to 0x0300 as of php 5.6.7, the extra cases of SQL_WCHAR and SQL_WVARCHAR come into effect, setting colfieldid to SQL_DESC_OCTET_LENGTH also for SQL_CHAR and SQL_VARCHAR due to a rather suspiscious fall-through strategy, that worked well for ODBCVER < 0x0300. However, not all odbc drivers allow for SQLColAttributes to be called with SQL_DESC_OCTET_LENGTH.

If SQLColAttributes fails, the effect is that strings lifted from subsequent calls to odbc_result become truncated, probably because of uninitialized variables, as the code doesn't even look at the return value of SQLColAttributes (same effect as in #68014, which appears to be fixed judging from looking at the code).

The correct fix would be to call SQLColAttribute instead of SQLColAttributes, if ODBCVER >= 0x0300, and to actually check the return value before using the result of the call (displaysize).

I have never figured out how to build my own php, so I have not tried to fix it directly in php_odbc.c, but I have tried to fix my odbc driver to support SQL_DESC_OCTET_LENGTH in SQLColAttributes, and that does indeed fix the problem.

Patches

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports

[2015-04-02 12:02 UTC] ab@php.net

-Status: Open +Status: Feedback -Assigned To: +Assigned To: ab

[2015-04-02 12:02 UTC] ab@php.net

Hi,

please supply a code snippet illustrating the broken behavior. Are you using the ADS driver, or which exactly?

Thanks.

[2015-04-02 14:07 UTC] php at mdjnet dot dk

-Status: Feedback +Status: Assigned

[2015-04-02 14:07 UTC] php at mdjnet dot dk

I am using a proprietary ODBC driver based on http://syware.com/products/odbc_driver_kit.php. I just tried to set up a scenario using Microsoft Access Driver (*.mdb), but it does allow to call SQLColAttributes with SQL_DESC_OCTET_LENGTH, so it did not have the problem.

A simple script showing the problem:

<html><body>
<?php	$db = odbc_connect('TEST','','');
	$res = odbc_exec($db,"select TNAME from Table1");
	if ($res)
		if (odbc_fetch_row($res))
		{
			$tname = odbc_result($res,'tname');
			print("tname = $tname\n");
		} ?>
</body></html>

using a simple mdb file with a table, Table1, containing a column, TNAME. The bug would cause the content of tname to be truncated somewhere - but I suppose you do need an ODBC driver NOT allowing SQLColAttributes with SQL_DESC_OCTET_LENGTH in order to see it. I *could* write a special edition of my odbc driver, displaying just this behaviour? I do believe that https://msdn.microsoft.com/en-us/library/ms711003 and https://msdn.microsoft.com/en-us/library/ms713558 documents the problem, but I guess we should be careful not to patch up php just to fit a special built odbc driver. Can't point you to any other driver triggering this problem though...

[2015-04-02 19:34 UTC] ab@php.net

-Status: Assigned +Status: Feedback

[2015-04-02 19:34 UTC] ab@php.net

Please check this snapshot http://windows.php.net/downloads/snaps/master/rf265928/ . I've integrated appropriate usage SQLColAttribute usage for ODBC 3.x and a couple of other compliance fixes.

With your snippet - I couldn't reproduce the behavior you describe with SQL Server or mdb driver. For this reason I've put the patches into master only, maybe yet. Maybe you should give me an exact CREATE TABLE and data.

The idea of using the ODBC 3.x APIs is actually good, I've also seen that there are many other APIs from 2.x used in ext/odbc. However the docs state some high compatibilty grade (with exception of some edge cases), so maybe it's also plausible to fix issues as they show up.

Thanks.

[2015-04-02 19:44 UTC] php at mdjnet dot dk

-Status: Feedback +Status: Assigned

[2015-04-02 19:44 UTC] php at mdjnet dot dk

I have now created a dummy odbc driver, capable of practically nothing, but enough to demonstrate the error. It can be downloaded from http://www.mdjnet.dk/testodbc.dll. I will leave it there, while this bug is open.

You will have to install it "manually", by editing the relevant registry keys. Create a DSN called TEST using the driver, and then you can simply run the script from my previous post. I am myself using Windows 7, apache 2.4.12 and of course php 5.6.7.

You will see the message "Allowed memory size of 134217728 bytes exhausted (tried to allocate 1996545750 bytes)" in the error log (because only the one column is queried with the script, so displaysize in php_odbc.c is uninitialized).

Then edit the script a little, use "select ID,TNAME from table1" instead. Now you will see the string "This is a v", it is cut to 11 characters (because displaysize is now set by the previous column to apparently 11), and if you query the driver by any other program, you will receive the string "This is a very, very, very, very, very ... long string indeed!". I made a very long string to make sure the truncation was visible, I could apparently have stopped at 12 characters. If you start the ODBC trace while running the script, you will see that SQLColAttributes returns -1 for 1013.

Let me know, if anything else is required to reproduce it.

[2015-04-02 20:40 UTC] ab@php.net

-Status: Assigned +Status: Feedback

[2015-04-02 20:40 UTC] ab@php.net

Yeah, I require really

- a mdb file or sql file to import
- which registry keys are to edit
- the exact php code

we call it "a self enclosed reproduce case" :) Otherwise it's really shooting in the sky.

I was adding a test http://git.php.net/?p=php-src.git;a=commitdiff;h=f26592846f8190fdab6cb148e484a4aeb032c096 based on your PHP example, however it doesn't repro on the usual drivers. Also, maybe it's already fixed in master, so worth to check that snap. If it is, one could think about backporting it into 5.5/6

Thanks.

[2015-04-02 20:55 UTC] php at mdjnet dot dk

-Status: Feedback +Status: Assigned

[2015-04-02 20:55 UTC] php at mdjnet dot dk

Try to shoot this into your registry (assuming you put the driver in c:\windows\system32, and that you have a 64 bit Windows, otherwise remove the Wow6432Node part):

Windows Registry Editor Version 5.00

[HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\ODBC\ODBCINST.INI\TestOdbc]
"UsageCount"=dword:00000001
"Setup"="C:\\Windows\\System32\\TestOdbc.dll"
"Driver"="C:\\Windows\\System32\\TestOdbc.dll"
"APILevel"=dword:00000001
"ConnectFunction"="YYN"
"DriverODBCVer"="03.00"
"FileExtns"=""
"FileUsage"=dword:00000000
"SQLLevel"=dword:00000000

[HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\ODBC\ODBCINST.INI\ODBC Drivers]
"TestOdbc"="Installed"

- and you should have a driver ready to configure a DSN for. No database, no nothing else required, the odbc driver has it all within it.

[2015-04-03 06:21 UTC] php at mdjnet dot dk

I just tried out the rf265928 snapshot, and it works fine. The odbc trace shows that now SQLColAttribute is called instead of SQLColAttributes.

[2015-04-05 10:27 UTC] ab@php.net

-Status: Assigned +Status: Feedback

[2015-04-05 10:27 UTC] ab@php.net

Please check the latest 5.5+ snapshots, the fixes was cherry picked.

Thanks.

[2015-04-05 12:35 UTC] php at mdjnet dot dk

-Status: Feedback +Status: Assigned

[2015-04-05 12:35 UTC] php at mdjnet dot dk

I have just tried out php-5.6-ts-windows-vc11-x86-rc585b48.zip, and it appears to working just fine with un-fixed odbc drivers.

[2015-04-06 15:23 UTC] ab@php.net

Thanks for checking, closing.

[2015-04-06 15:23 UTC] ab@php.net

-Status: Assigned +Status: Closed

[2015-04-15 13:37 UTC] frederic dot marchal at wowtechnology dot com

Related To: Bug #68350

[2015-04-19 07:58 UTC] frederic dot marchal at wowtechnology dot com

Related To: Bug #68350