Edit report at https://bugs.php.net/bug.php?id=80460&edit=1
ID: 80460
Comment by: mirish at ibm dot com
Reported by: mirish at ibm dot com
Summary: ODBC doesn't account for SQL_NO_TOTAL indicator,
causing segmentation fault
Status: Analyzed
Type: Bug
Package: ODBC related
Operating System: RHEL 7.9
PHP Version: Irrelevant
Assigned To: cmb
Block user comment: N
Private report: N
New Comment:
I have tested the patch and it does work (no more segfaults!), but I have a few thoughts:
1.
I'm not sure just throwing an error is the best solution. If the driver returns SQL_NO_TOTAL,
there is still good data in the buffer, it just isn't ALL of the data (and the driver
doesn't want to calculate how much is left, probably because it would have to translate to a
different character encoding). The string data in the buffer should be null-terminated[1], so using
ZVAL_STRING instead of ZVAL_STRINGL should retrieve all of the good data from the buffer. Its
possible this would be a good change for ALL ZVAL_STRINGLs in the code, but to minimize the amount
of regressions added to the code I think it would be fine adding to the if-else blocks you've
already created. I think users might still want to know that not all data was returned, so keeping
the warning is probably a good idea:
...
} else if (result->values[i].vallen == SQL_NO_TOTAL) {
ZVAL_STRING(&tmp, buf);
php_error_docref(NULL, E_WARNING, "Cannot get all data of column #%d (driver
cannot determine length)", i + 1, rc);
} else {
...
2.
Although the case of SQL_NO_TOTAL should still be handled, I think most of it can be avoided if the
variable charextraalloc is always set to true, multiplying whatever is returned from
SQL_DESC_OCTET_LENGTH by 4. Currently it only sets this variable to true in a few cases, but it
should probably be broadened.
I have tested several ODBC 3.0-compliant drivers and they don't seem to treat
SQL_DESC_OCTET_LENGTH in a standardized way when the schema/table/field character encoding differs
from the client character encoding. For instance, a field with a character encoding of Windows-1251
could have a CHAR(1) field that holds the â character in a single byte (0x86). If the client
wants data returned in UTF-8, this expands to become 0xE2 0x80 0xA0, requiring 3 bytes. Like I said,
drivers seem to handle this case differently: FreeTDS and the IBM i Access driver would return 1,
while the MySQL driver actually checks to see the maximum UTF-8 expansion and returns 3. The safest
solution, even if it wastes a tiny amount of space in the buffer for a short amount of time, is to
multiply by 4.
Always multiplying by 4 SHOULD ensure enough buffer space so that all of the data is returned and so
SQL_NO_TOTAL isn't encountered. In my test case, when I set charextraalloc to always be true,
my queries return data perfectly.
3.
I am not having much luck running the ODBC tests, I pass in the environment variables (or even
export them) and try to run it, but 20 of the 21 tests are skipped as "could not connect".
Will continue to try. I don't think there are any tests that check for SQL_NO_TOTAL, and since
it happens in a very particular circumstance it would be hard to force, but I can write some tests
that at least change the StrLen_or_IndPtr value after it is returned to force the code to take the
SQL_NO_TOTAL path.
[1]: There is an option in SQLSetEnvAttr called SQL_ATTR_OUTPUT_NTS that controls whether character
data returned is null-terminated. It looks like you CAN set it to SQL_FALSE, but the docs note:
"A call to SQLSetEnvAttr to set it to SQL_FALSE returns SQL_ERROR and SQLSTATE HYC00 (Optional
feature not implemented)." Not sure if that is MSSQL-specific, but I have never known any one
to try to use this feature.
Previous Comments:
------------------------------------------------------------------------
[2021-03-26 14:25:59] cmb@php.net
@mirish, could you please test the provided patch[1]? It would
also be great if you could run the test suite before and after
applying the patch, to see whether we already have tests covering
this scenario. If you build from source, you can run the test
suite by running
make test TESTS=ext/odbc/tests
You need to set the environment variables ODBC_TEST_DSN,
ODBC_TEST_USER and ODBC_TEST_PASS for that to work.
If the existing tests do not (sufficently) cover this scenario,
it would be great if you could provide (an) additional test(s).
[1] <https://github.com/php/php-src/pull/6809>
------------------------------------------------------------------------
[2021-03-26 14:20:29] cmb@php.net
The following pull request has been associated:
Patch Name: Fix #80460: ODBC doesn't account for SQL_NO_TOTAL indicator
On GitHub: https://github.com/php/php-src/pull/6809
Patch: https://github.com/php/php-src/pull/6809.patch
------------------------------------------------------------------------
[2021-03-26 10:16:36] cmb@php.net
Thanks for the clarification! Indeed, these SQL_NO_TOTAL checks
are necessary.
------------------------------------------------------------------------
[2021-03-25 20:04:34] mirish at ibm dot com
Ok, a little clarification: It looks like my particular error is generated from the StrLen_or_IndPtr
bound from SQLBindCol: https://docs.microsoft.com/en-us/sql/odbc/reference/syntax/sqlbindcol-function?view=sql-server-ver15
The valid values are still:
* The length of the data available to return
* SQL_NO_TOTAL
* SQL_NULL_DATA
Then, when odbc_fetch_row is called, SQLExtendedFetch gets called (in my particular test, but
SQLFetch and SQLGetData can equally return the same thing), populates that indicator bound in
SQLBindCol, and SQL_NO_TOTAL is returned to that StrLen_or_IndPtr, which is mapped in the php_odbc.c
code to result->values[field_ind].vallen.
Then, the code tries to generate a string from that length (-4), causing the seg fault.
Backtrace:
#0 0x00007f0d54ed9474 in __memcpy_ssse3_back () from /usr/lib64/libc.so.6
#1 0x0000000000591919 in zend_string_init (str=0x7f0d54491028 "",
len=18446744073709551612, persistent=0)
at /home/mirish/php-src/Zend/zend_string.h:157
#2 0x0000000000596dc1 in zif_odbc_result (execute_data=0x7f0d54414150, return_value=0x7f0d54414120)
at /home/mirish/php-src/ext/odbc/php_odbc.c:2224
#3 0x000000000085cbb1 in ZEND_DO_ICALL_SPEC_RETVAL_USED_HANDLER () at
/home/mirish/php-src/Zend/zend_vm_execute.h:1313
#4 0x00000000008bc92c in execute_ex (ex=0x7f0d54414020) at
/home/mirish/php-src/Zend/zend_vm_execute.h:53564
#5 0x00000000008c09fa in zend_execute (op_array=0x7f0d5447e400, return_value=0x0)
at /home/mirish/php-src/Zend/zend_vm_execute.h:57664
#6 0x00000000007ef9fc in zend_execute_scripts (type=8, retval=0x0, file_count=3)
at /home/mirish/php-src/Zend/zend.c:1663
#7 0x000000000075bba0 in php_execute_script (primary_file=0x7ffd3adb2620) at
/home/mirish/php-src/main/main.c:2619
#8 0x00000000008c31c6 in do_cli (argc=2, argv=0x2c11700) at
/home/mirish/php-src/sapi/cli/php_cli.c:961
#9 0x00000000008c4105 in main (argc=2, argv=0x2c11700) at
/home/mirish/php-src/sapi/cli/php_cli.c:1352
And for good measure, here is the ODBC trace:
...
[ODBC][88259][1616702310.287337][SQLExecDirect.c][240]
Entry:
Statement = 0x1f66a90
SQL = [SELECT * FROM MIRISH.UTF8TEST][length = 29 (SQL_NTS)]
[ODBC][88259][1616702310.468968][SQLExecDirect.c][521]
Exit:[SQL_SUCCESS]
[ODBC][88259][1616702310.469037][SQLNumResultCols.c][156]
Entry:
Statement = 0x1f66a90
Column Count = 0x7effad458730
[ODBC][88259][1616702310.469087][SQLNumResultCols.c][251]
Exit:[SQL_SUCCESS]
Count = 0x7effad458730 -> 1
[ODBC][88259][1616702310.469128][SQLColAttribute.c][294]
Entry:
Statement = 0x1f66a90
Column Number = 1
Field Identifier = SQL_DESC_NAME
Character Attr = 0x7effad45c140
Buffer Length = 256
String Length = 0x7fff4dbce140
Numeric Attribute = (nil)
[ODBC][88259][1616702310.469188][SQLColAttribute.c][709]
Exit:[SQL_SUCCESS]
[ODBC][88259][1616702310.469217][SQLColAttribute.c][294]
Entry:
Statement = 0x1f66a90
Column Number = 1
Field Identifier = SQL_DESC_CONCISE_TYPE
Character Attr = (nil)
Buffer Length = 0
String Length = (nil)
Numeric Attribute = 0x7effad45c250
[ODBC][88259][1616702310.469252][SQLColAttribute.c][709]
Exit:[SQL_SUCCESS]
[ODBC][88259][1616702310.469280][SQLColAttribute.c][294]
Entry:
Statement = 0x1f66a90
Column Number = 1
Field Identifier = SQL_DESC_OCTET_LENGTH
Character Attr = (nil)
Buffer Length = 0
String Length = (nil)
Numeric Attribute = 0x7fff4dbce138
[ODBC][88259][1616702310.469309][SQLColAttribute.c][709]
Exit:[SQL_SUCCESS]
[ODBC][88259][1616702310.469338][SQLBindCol.c][236]
Entry:
Statement = 0x1f66a90
Column Number = 1
Target Type = 1 SQL_CHAR
Target Value = 0x7effad491028
Buffer Length = 2
StrLen Or Ind = 0x7effad45c248
[ODBC][88259][1616702310.469382][SQLBindCol.c][344]
Exit:[SQL_SUCCESS]
[ODBC][88259][1616702310.469452][SQLExtendedFetch.c][166]
Entry:
Statement = 0x1f66a90
Fetch Type = 1
Row = 1
PcRow = 0x7fff4dbce1a0
Row Status = 0x7fff4dbce190
[ODBC][88259][1616702310.560382][SQLExtendedFetch.c][339]
Exit:[SQL_SUCCESS]
------------------------------------------------------------------------
[2021-03-25 17:46:25] mirish at ibm dot com
Last comment should read:
result->values[i].vallen == SQL_NO_TOTAL
Similar to how it checks for SQL_NULL_DATA.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=80460
--
Edit this bug report at https://bugs.php.net/bug.php?id=80460&edit=1