|
|
php.net | support | documentation | report a bug | advanced search | search howto | statistics | random bug | login |
[2021-12-02 18:30 UTC] calvin at cmpct dot info
Description: ------------ (was kinda wanting to wait until the transition to GH issues, but deciding to post this now... let me know if i should) It seems PDO_ODBC doesn't handle situations where if characters are converted as part of a data bind. For example, if the CHAR(30) column on the database stores as one encoding and it gets converted to something requiring more bytes to represent, there's seemingly a possibility of a buffer overflow (what looks like to me, anyways). The example script has one such example; an é takes one byte to represent in some encodings, and two in UTF-8. Db2 is a pathological case because most columns are stored as some kind of EBCDIC encoding and gets converted to usually UTF-8 on the client, but it's quite possible for other databases too (i.e. ISO-8859-1 columns to UTF-8). The example I posted would probably trigger the same issue if the driver is converting. Looking at the unixODBC trace a user provided, I can see PHP just allocates what the driver claims is the CHAR(n) size plus one. PHP PDO_ODBC: ``` [ODBC][24294][1638369671.619132][SQLDescribeCol.c][504] Exit:[SQL_SUCCESS] Column Name = [VARTE1] Data Type = 70000000009b318 -> 1 Column Size = fffffffffffce78 -> 30 (64 bits) Decimal Digits = NULLPTR Nullable = NULLPTR [ODBC][24294][1638369671.619175][SQLColAttribute.c][294] Entry: Statement = 180847ad0 Column Number = 1 Field Identifier = SQL_DESC_DISPLAY_SIZE Character Attr = 0 Buffer Length = 0 String Length = 0 Numeric Attribute = fffffffffffce70 [ODBC][24294][1638369671.619219][SQLColAttribute.c][709] Exit:[SQL_SUCCESS] [ODBC][24294][1638369671.619261][SQLBindCol.c][245] Entry: Statement = 180847ad0 Column Number = 1 Target Type = 1 SQL_CHAR Target Value = 700000000056540 Buffer Length = 31 StrLen Or Ind = 70000000009b310 [ODBC][24294][1638369671.619304][SQLBindCol.c][353] Exit:[SQL_SUCCESS] ``` For comparison, Node ODBC library: ``` [ODBC][24271][1638369403.771225][SQLDescribeCol.c][504] Exit:[SQL_SUCCESS] Column Name = [VARTE1] Data Type = 1805796a4 -> 1 Column Size = 1805796a8 -> 30 (64 bits) Decimal Digits = 1805796b0 -> 0 Nullable = 1805796c0 -> 0 [ODBC][24271][1638369403.771312][SQLBindCol.c][245] Entry: Statement = 180552e70 Column Number = 1 Target Type = 1 SQL_CHAR Target Value = 180583130 Buffer Length = 121 StrLen Or Ind = 1805437d0 [ODBC][24271][1638369403.771392][SQLBindCol.c][353] Exit:[SQL_SUCCESS] ``` It looks to be overallocating, per: https://github.com/markdirish/node-odbc/blob/master/src/odbc_connection.cpp#L1659 ibm_db2 actually has a flag (i5_dbcs_alloc) in case of these situations. It is a bit crude, but seeing other ODBC consumers do it is kind of suggesting to me it may be a valid solution. The other option might be to reallocate more aggressively if the buffer is inadequate. I'm unsure how tricky that'd be to implement. The annoying option for users but simplest to prevent crashes in PHP is to emit a warning if the driver is trying to return more that what PHP allocated. Test script: --------------- <?php // SQL to repro create table calvin.bigchars( x char(30) ); // character that must be representable as a single char in DB but expanded on client (i.e. accented e in ISO-8859 or EBCDIC is multiple bytes in UTF-8) insert into calvin.bigchars values('éééééééééééééééééééééééééééééé'); // 30 insert into calvin.bigchars values('ééééééééééééééé'); // 15 insert into calvin.bigchars values('ééééé'); // 5 // End SQL //Connect to IBM i $user = 'user'; $pw = 'password'; // CCSID 1208 for UTF-8, 819 for ISO-8859-1 $dsn = 'Driver={IBM i Access ODBC Driver};System=127.0.0.1;AlwaysCalculateResultLength=1;CCSID=1208'; $persistence = false; // create pdo odbc connection try { $pdo_odbc = new PDO("odbc:$dsn", $user, $pw, array( PDO::ATTR_PERSISTENT => $persistence, PDO::ATTR_ERRMODE => PDO::ERRMODE_WARNING, )); } catch (PDOException $e) { die($e->getMessage()); } $sql = "select * from calvin.bigchars"; $pdoStmt = $pdo_odbc->prepare($sql); echo "before PDO\n"; var_dump($pdoStmt->execute()); echo "after PDO\n"; while($record = $pdoStmt->fetch(PDO::FETCH_ASSOC)){ echo "Record\n"; var_dump($record["X"]); } Actual result: -------------- before PDO bool(true) after PDO Record string(60) "éééééééééééééééT`� ��sql" Record string(45) "éééééééééééééééT`� " Record string(35) "ééééé " PatchesPull RequestsHistoryAllCommentsChangesGit/SVN commits
|
|||||||||||||||||||||||||||||||||||||
Copyright © 2001-2024 The PHP GroupAll rights reserved. |
Last updated: Thu Nov 21 17:01:32 2024 UTC |
I can confirm that PDO::ODBC_ATTR_ASSUME_UTF8 and using the procedural ODBC extension have the same results. (Well, the garbage on the end is different, but it's still garbage and the string is still the same length.) Code for the procedural ODBC version (the attrib on PDO ver is trivial so not including): ``` $odbc = odbc_connect($dsn, $user, $pw); $sql = "select cast(x as char(150)) as X from calvin.bigchars"; $stmt = odbc_prepare($odbc, $sql); echo "before ODBC\n"; var_dump(odbc_execute($stmt)); echo "after ODBC\n"; while($record = odbc_fetch_array($stmt)){ echo "Record\n"; var_dump($record["X"]); } ```Sorry for the late reply! From what I can tell, there may be two different issues here: (a) PHP does not allocate sufficient space for the data, and (b) the driver may deliver fewer data than PHP expects, resulting in arbitrary memory contents to be given to userland. Let's focus on (b) for now, since that should be easy to fix by zeroing the buffers before fetching. Could you please try the following patch for ODBC (PDO_ODBC could likely get a similar fix): ext/odbc/php_odbc.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ext/odbc/php_odbc.c b/ext/odbc/php_odbc.c index d4de4ec75b..d0ee19aa07 100644 --- a/ext/odbc/php_odbc.c +++ b/ext/odbc/php_odbc.c @@ -1381,6 +1381,12 @@ static void php_odbc_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, int result_type) RETURN_FALSE; } + for (i = 0; i < result->numcols; i++) { + if (result->values[i].value != NULL) { + memset(result->values[i].value, 0, result->values[i].vallen); + } + } + #ifdef HAVE_SQL_EXTENDED_FETCH if (result->fetch_abs) { if (rownum > 0) { Something else that may be worth investigating is the usage of SQL_LONGVARCHAR columns (or respective casts), if something like that is supported by the DB and the driver. Such columns are not bound, but rather SQLGetData() is used to retrieve the data, and the driver is supposed to report the length of the transferred data in the StrLen_or_IndPtr argument (so in this case neither SQLDescribeCol() nor SQLColAttribute() are used to compute the length).> […], just chuffed at the issue tracker that it got auto-closed. Yeah, that is a bit unfortunate. If the status is set to "feedback", the bug is automatically closed on the next but one Sunday, even if feedback has been provided. You, as bug reporter, should be able to change the status back to "open", though ("re-opened" doesn't work, IIRC). > but I don't notice any different behaviour Hmm, does the *driver* fill the buffers with some garbage at the end, in this case? I'll have a closer look tomorrow; it seems using SQLGetData() always (for testing purposes) might be a way to proceed.