From f105b9f5cd7b0f32cbf373ab951c30e97bb611b4 Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 25 Oct 2024 14:36:38 +0200 Subject: [PATCH] Fix GH-16450: PDO_ODBC can inject garbage into field values A previous bug fix[1] relied on ODBC drivers to properly count down the `StrLen_or_IndPtr` argument for consecutive calls to `SQLGetData()`. Apparently, not all drivers handle this correctly, so we cannot assert they do. Instead we fall back to the old behavior for drivers which would violate the assertion. A test against SQLServer (which we currently use in CI) would not make sense, since the respective drivers do not exhibit that behavior. Instead we target the regression test especially to a MS Access database. Since there is apparently no way to easily create an MS Access database programmatically, we commit a minimal empty DB which is used for the regression test, and could also be used by other test cases. [1] --- ext/pdo_odbc/odbc_stmt.c | 7 ++++--- ext/pdo_odbc/tests/gh16450.phpt | 35 ++++++++++++++++++++++++++++++++ ext/pdo_odbc/tests/test.mdb | Bin 0 -> 77824 bytes 3 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 ext/pdo_odbc/tests/gh16450.phpt create mode 100644 ext/pdo_odbc/tests/test.mdb diff --git a/ext/pdo_odbc/odbc_stmt.c b/ext/pdo_odbc/odbc_stmt.c index bd4a2f6162d09..1df4e22571a76 100644 --- a/ext/pdo_odbc/odbc_stmt.c +++ b/ext/pdo_odbc/odbc_stmt.c @@ -689,11 +689,12 @@ static int odbc_stmt_get_col(pdo_stmt_t *stmt, int colno, zval *result, enum pdo /* read block. 256 bytes => 255 bytes are actually read, the last 1 is NULL */ rc = SQLGetData(S->stmt, colno+1, C->is_unicode ? SQL_C_BINARY : SQL_C_CHAR, buf2, 256, &C->fetched_len); - /* adjust `used` in case we have length info from the driver */ + /* adjust `used` in case we have proper length info from the driver */ if (orig_fetched_len >= 0 && C->fetched_len >= 0) { SQLLEN fixed_used = orig_fetched_len - C->fetched_len; - ZEND_ASSERT(fixed_used <= used + 1); - used = fixed_used; + if (fixed_used <= used + 1) { + used = fixed_used; + } } /* resize output buffer and reassemble block */ diff --git a/ext/pdo_odbc/tests/gh16450.phpt b/ext/pdo_odbc/tests/gh16450.phpt new file mode 100644 index 0000000000000..e29d7672ee74b --- /dev/null +++ b/ext/pdo_odbc/tests/gh16450.phpt @@ -0,0 +1,35 @@ +--TEST-- +GH-16450 (PDO_ODBC can inject garbage into field values) +--EXTENSIONS-- +pdo_odbc +--SKIPIF-- + +--FILE-- +exec("CREATE TABLE gh16450 (Id INT, MyLongText LONGCHAR)"); +$pdo->exec(sprintf("INSERT INTO gh16450 VALUES (1, '%s')", str_repeat("_", 2048))); +$pdo->exec(sprintf("INSERT INTO gh16450 VALUES (1, '%s')", str_repeat("_", 2049))); + +$stmt = $pdo->query("SELECT MyLongText FROM gh16450"); +var_dump($stmt->fetchColumn(0)); +var_dump($stmt->fetchColumn(0)); +?> +--CLEAN-- +exec("DROP TABLE gh16450"); +?> +--EXPECT-- +string(2048) "________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________" +string(2049) "_________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________________" diff --git a/ext/pdo_odbc/tests/test.mdb b/ext/pdo_odbc/tests/test.mdb new file mode 100644 index 0000000000000000000000000000000000000000..836d813e498396d1808ebc9a54c75541bad47b97 GIT binary patch literal 77824 zcmeI5U1%KF701u)j-=Io%C`bLsN&_Fo2d(zdif&tNUJi_xC>;d*`pee!lPD&%AbM`JpSH{o;qu{p;eD zPrl#cK6AY1&L{7E@AY?I9__p7;5&O>zUTJ0&OY+ZwsW8Q>7{c&%G~zD|D1p0uH|2z z`_|-_fAG=8@7(e9iQnCJSMMMH{)dUHLEDAC^H1!`U;W_j@}G{hiX7Ox4?E#O0wh2J zBtQZrKmsH{0wh2JB(N0(90%?mA2%!k#~Jt4-`680zwpoaw#_REkN^pg011!)36KB@ zkN^pg011%5b|Uamu$d#}{3WN>-0F%PL(V-DnijV*g|!2kyXdcFp7=9&d^nJI->w(YiEcR6rrr4z=w$q1)hPDTpSzHH4M9&UA3#A zwQAsgyTMO{+qE|$M&d@>&Fei#s9IUUD^-;Ptu=!5s*_ehy1It7TyKLX2W4L7@R*P> zS&*Xi$|#;krHu15dT$PUCNQ#s*D6i+9+guv9`lzHc^sN1ByV~ihIAh5WU5-SG9+IyyRKl?!w@Mm!^C@$;bPi_Kj7!chVUGTxT)MonV&~``(VJ^-u>J|F3fDC+ zRaQM>nd!3pZmF|5;@d7RX1AF2Qn;pZ79@jfJILkHo=^%>`G3JzOErs2S; z!>SHr5sqP59z#prS#<_mWdzT?#<9by#w%0*rv6_F7OAVR$G6ROBS)-$B|1$88ZRV3 z0wh2JBtQZrKmsH{0wh2J+mArXxm?g)6_CqJI=glLKl1JH^tIcFP&D9}!hzVNl+KTx zcAI0?kb%khx6~8^=sc5^ZiV7x#GnOP=`gjwR?volCW#*Bv29&}ruL1%tri2sf{kMg zjq>pwh%qeSp>c&W($F^Dtx4)rCdqsn$?a*AP|j4--L;AAY&B8cg@{6wn%&!iPWxx%cX`GB+gkbqZ0wh2J zBtQZrKmsH{0wh2J%}5~SylMQ&yHVtybS14HA6q<A8lY6KcNXgH&_E+`R3609xB0{&IU zLq)+D{Qi@O3CyCLJd1+z3L*qrec<7mlob&p_^K(bk4D>`WNpG7X3SxD{*%W0*Yfo- zw8q!WTYpqi*Y2t=Ex%s~;}fS*-ad_}MY24-HlkpwAX2!OrVa##f87|ba}d0-;)d5W z=0x8X5zoN~85)kZoi64?B_?)G6f`n#xHx`c_MkGo*Lo&m8&#QUG?4v2tgoFSEZ{)` zBtQZrKmsH{0wh2JBtQZ?mw@B^BdU1Er}Xvv|GNAu-p{kOPLXac=|{#EAM!^IeJT0|yT;?$yX^=pK8r0a4)xFW5A@Fr&**<) zCC7G74?o&Jdu+Ts^QB{xR*-!~nr~s(>GU>NLC}0mXb}mJ011!)36KB@kN^pg011!)2{ao4 z=Kq`Rj?x|yAOR8}0TLhq5+DH*AOR9+1_ImW7dX3{fu`D^`G0)mn+FMy011!)36KB@ zkN^pg011!)36MY&6QKX!#5b3wkpKyh011!)36KB@kN^pg011#lNPzx7-wGr^0wh2J zBtQZrKmsH{0wh2JB+&c>9Ony#RA^>KUiHWQod3EvpVbz8dfj*AZ{MAD-|Bh4J9>s-X;4Pwtelp^G9f2rR2F1f=4B2& zQ)r#Sb8t_u)IWNpQ*siBl=PRpuVH~ z^Ma>$S&yPz_5bW&@L%!I```1Q^r!rzevkhdf4AT6f9SpEU9A36pdHUE36KB@kN^pg z011!)36KB@kN^o>F9NQp6CVV=1YLQ_BHZA+Cgu>VuIbWH#LYOPQEjdX(bRNlP^G2| zJw^3{6BBJ$E8+`wMd-2)|C%n{mm_sut8nCl18cTgGfI8^no;%}`@H0ex>c!DU)!Z- HuNn1UitQGQ literal 0 HcmV?d00001