issues with Zend_Db_Statement::_stripQuoted

[Vincent de Lau]

Hi all,

Last week we ran in an issue with a segfault caused by the preg_replace
statements in Zend_Db_Statements::_stripQuoted (issues ZF-5063[1] and
ZF-7585[2]). While trying to find a work-around, I discovered that this
function is broken. The fix for issue ZF-3025[3] seems to be applied wrong
(r9727).

The fix for my issue could be to modify the regular expression. Instead of
the repetition, my replacement relies on assertions. During initial testing
(running a 1MB query) it seems that this would not rely on the stack too
much, reducing the chance of a segfault. This would need to be tested
further. In the patch below, I restored the original replacement of quoted
identifiers that was lost in r9727.

However, there are still some issues:
- In my regexp, I assume that a quote can be escaped by repeating the quote
or by prefixing it with a backslash. A backslash itself can be escaped by
repeating it. I'm not sure if these are safe assumptions for all RDBMS's.
- The patch should be tested thoroughly, although I'm quite confident about
it.
- MySQL accepts both single and double quotes for values. This is not
accounted for, nor was it in the old version. I'm not sure how this is for
other RDBMS's. Would it be safe to strip out all quote style: single ('),
double (") and backtick (`)? I could imagine that for most systems this
would be OK. Individual adapters could always override the default function
and provide their own.

Index: Statement.php
===================================================================
--- Statement.php       (revision 17563)
+++ Statement.php       (working copy)
@@ -164,30 +164,22 @@
         $d = $this->_adapter->quoteIdentifier('a');
         $d = $d[0];

-        // get the value used as an escaped delimited id quote,
-        // e.g. \" or "" or \`
-        $de = $this->_adapter->quoteIdentifier($d);
-        $de = substr($de, 1, 2);
-        $de = str_replace('\\', '\\\\', $de);
-
         // get the character for value quoting
         // this should be '
         $q = $this->_adapter->quote('a');
         $q = $q[0];

-        // get the value used as an escaped quote,
-        // e.g. \' or ''
-        $qe = $this->_adapter->quote($q);
-        $qe = substr($qe, 1, 2);
-        $qe = str_replace('\\', '\\\\', $qe);
-
         // get a version of the SQL statement with all quoted
         // values and delimited identifiers stripped out
-        // remove "foo\"bar"
-        $sql = preg_replace("/$q($qe|\\\\{2}|[^$q])*$q/", '', $sql);
-        // remove 'foo\'bar'
+        // remove quoted identifiers
+        if (!empty($d)) {
+            $rx =
"{$d}.*?(?<!(((?<![{$d}\\\\]){$d})|((?<!\\\\)\\\\))){$d}(?!{$d})";
+            $sql = preg_replace("/$rx/", '', $sql);
+        }
+        // remove quoted values
         if (!empty($q)) {
-            $sql = preg_replace("/$q($qe|[^$q])*$q/", '', $sql);
+            $rx =
"{$q}.*?(?<!(((?<![{$q}\\\\]){$q})|((?<!\\\\)\\\\))){$q}(?!{$q})";
+            $sql = preg_replace("/$rx/", '', $sql);
         }

         return $sql;
===EOF

[1] http://framework.zend.com/issues/browse/ZF-5063
[2] http://framework.zend.com/issues/browse/ZF-7585
[3] http://framework.zend.com/issues/browse/ZF-3025

Vincent de Lau
 [hidden email]

[Vincent de Lau]

> -----Original Message-----
> From: Vincent de Lau [mailto:[hidden email]]
> Sent: Sunday, September 20, 2009 3:42 AM
> To: [hidden email]
> Subject: [fw-db] issues with Zend_Db_Statement::_stripQuoted
>
> Hi all,
>
> Last week we ran in an issue with a segfault caused by the preg_replace
> statements in Zend_Db_Statements::_stripQuoted (issues ZF-5063[1] and
> ZF-7585[2]). While trying to find a work-around, I discovered that this
> function is broken. The fix for issue ZF-3025[3] seems to be applied
> wrong
> (r9727).
>
> The fix for my issue could be to modify the regular expression. Instead
> of
> the repetition, my replacement relies on assertions. During initial
> testing
> (running a 1MB query) it seems that this would not rely on the stack
> too
> much, reducing the chance of a segfault. This would need to be tested
> further. In the patch below, I restored the original replacement of
> quoted
> identifiers that was lost in r9727.

A simpler fix might be the next patch. Instead of assertions, it tries to
limit stack usage by 'eating' similar subpatterns.

Index: Statement.php
===================================================================
--- Statement.php       (revision 17563)
+++ Statement.php       (working copy)
@@ -184,10 +184,10 @@
         // get a version of the SQL statement with all quoted
         // values and delimited identifiers stripped out
         // remove "foo\"bar"
-        $sql = preg_replace("/$q($qe|\\\\{2}|[^$q])*$q/", '', $sql);
+        $sql = preg_replace("/$d($de+|\\\\{2}+|[^$d]+)*$d/", '', $sql);
         // remove 'foo\'bar'
         if (!empty($q)) {
-            $sql = preg_replace("/$q($qe|[^$q])*$q/", '', $sql);
+            $sql = preg_replace("/$q($qe+|\\\\{2}+|[^$q]+)*$q/", '', $sql);
         }

         return $sql;

Vincent de Lau
 [hidden email]