<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[BuddyPress][9073] trunk: Better detection for false positives in meta SQL filters.</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta" style="font-size: 105%">
<dt style="float: left; width: 6em; font-weight: bold">Revision</dt> <dd><a style="font-weight: bold" href="http://buddypress.trac.wordpress.org/changeset/9073">9073</a><script type="application/ld+json">{"@context":"http://schema.org","@type":"EmailMessage","description":"Review this Commit","action":{"@type":"ViewAction","url":"http://buddypress.trac.wordpress.org/changeset/9073","name":"Review Commit"}}</script></dd>
<dt style="float: left; width: 6em; font-weight: bold">Author</dt> <dd>boonebgorges</dd>
<dt style="float: left; width: 6em; font-weight: bold">Date</dt> <dd>2014-10-08 14:14:29 +0000 (Wed, 08 Oct 2014)</dd>
</dl>
<pre style='padding-left: 1em; margin: 2em 0; border-left: 2px solid #ccc; line-height: 1.25; font-size: 105%; font-family: sans-serif'>Better detection for false positives in meta SQL filters.
Our wrappers for WP's _metadata() functions require some filtering of the SQL
string (to change a column name and, in the case of xprofile, to add an
'object_type' clause). Our str_replace() logic is too generous, creating the
possibility of matching quoted text, as when the meta value contains the string
'WHERE'.
This changeset modifies the filters so that quoted content is swapped out with
placeholders before we run our search-and-replace.
See <a href="http://buddypress.trac.wordpress.org/ticket/5919">#5919</a>.
Props tometzky for feedback.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunksrcbpcorebpcorefiltersphp">trunk/src/bp-core/bp-core-filters.php</a></li>
<li><a href="#trunksrcbpxprofilebpxprofilefiltersphp">trunk/src/bp-xprofile/bp-xprofile-filters.php</a></li>
<li><a href="#trunktestsphpunittestcasesxprofilefunctionsphp">trunk/tests/phpunit/testcases/xprofile/functions.php</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunksrcbpcorebpcorefiltersphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/bp-core/bp-core-filters.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/bp-core/bp-core-filters.php 2014-10-07 01:57:22 UTC (rev 9072)
+++ trunk/src/bp-core/bp-core-filters.php 2014-10-08 14:14:29 UTC (rev 9073)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -603,7 +603,25 @@
</span><span class="cx" style="display: block; padding: 0 10px"> * @return string
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> function bp_filter_metaid_column_name( $q ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- return str_replace( 'meta_id', 'id', $q );
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ /*
+ * Replace quoted content with __QUOTE__ to avoid false positives.
+ * This regular expression will match nested quotes.
+ */
+ $quoted_regex = "/'[^'\\\\]*(?:\\\\.[^'\\\\]*)*'/s";
+ preg_match_all( $quoted_regex, $q, $quoted_matches );
+ $q = preg_replace( $quoted_regex, '__QUOTE__', $q );
+
+ $q = str_replace( 'meta_id', 'id', $q );
+
+ // Put quoted content back into the string.
+ if ( ! empty( $quoted_matches[0] ) ) {
+ for ( $i = 0; $i < count( $quoted_matches[0] ); $i++ ) {
+ $quote_pos = strpos( $q, '__QUOTE__' );
+ $q = substr_replace( $q, $quoted_matches[0][ $i ], $quote_pos, 9 );
+ }
+ }
+
+ return $q;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span></span></pre></div>
<a id="trunksrcbpxprofilebpxprofilefiltersphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/src/bp-xprofile/bp-xprofile-filters.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/src/bp-xprofile/bp-xprofile-filters.php 2014-10-07 01:57:22 UTC (rev 9072)
+++ trunk/src/bp-xprofile/bp-xprofile-filters.php 2014-10-08 14:14:29 UTC (rev 9073)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -335,18 +335,28 @@
</span><span class="cx" style="display: block; padding: 0 10px"> function bp_xprofile_filter_meta_query( $q ) {
</span><span class="cx" style="display: block; padding: 0 10px"> global $wpdb;
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ $raw_q = $q;
+
+ /*
+ * Replace quoted content with __QUOTE__ to avoid false positives.
+ * This regular expression will match nested quotes.
+ */
+ $quoted_regex = "/'[^'\\\\]*(?:\\\\.[^'\\\\]*)*'/s";
+ preg_match_all( $quoted_regex, $q, $quoted_matches );
+ $q = preg_replace( $quoted_regex, '__QUOTE__', $q );
+
</ins><span class="cx" style="display: block; padding: 0 10px"> // Get the first word of the command
</span><span class="cx" style="display: block; padding: 0 10px"> preg_match( '/^(\S+)/', $q, $first_word_matches );
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> if ( empty( $first_word_matches[0] ) ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- return $q;
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ return $raw_q;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> // Get the field type
</span><span class="cx" style="display: block; padding: 0 10px"> preg_match( '/xprofile_(group|field|data)_id/', $q, $matches );
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> if ( empty( $matches[0] ) || empty( $matches[1] ) ) {
</span><del style="background-color: #fdd; text-decoration:none; display:block; padding: 0 10px">- return $q;
</del><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ return $raw_q;
</ins><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> switch ( $first_word_matches[0] ) {
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -415,5 +425,13 @@
</span><span class="cx" style="display: block; padding: 0 10px"> break;
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span><span class="cx" style="display: block; padding: 0 10px">
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ // Put quoted content back into the string.
+ if ( ! empty( $quoted_matches[0] ) ) {
+ for ( $i = 0; $i < count( $quoted_matches[0] ); $i++ ) {
+ $quote_pos = strpos( $q, '__QUOTE__' );
+ $q = substr_replace( $q, $quoted_matches[0][ $i ], $quote_pos, 9 );
+ }
+ }
+
</ins><span class="cx" style="display: block; padding: 0 10px"> return $q;
</span><span class="cx" style="display: block; padding: 0 10px"> }
</span></span></pre></div>
<a id="trunktestsphpunittestcasesxprofilefunctionsphp"></a>
<div class="modfile"><h4 style="background-color: #eee; color: inherit; margin: 1em 0; padding: 1.3em; font-size: 115%">Modified: trunk/tests/phpunit/testcases/xprofile/functions.php</h4>
<pre class="diff"><span>
<span class="info" style="display: block; padding: 0 10px; color: #888">--- trunk/tests/phpunit/testcases/xprofile/functions.php 2014-10-07 01:57:22 UTC (rev 9072)
+++ trunk/tests/phpunit/testcases/xprofile/functions.php 2014-10-08 14:14:29 UTC (rev 9073)
</span><span class="lines" style="display: block; padding: 0 10px; color: #888">@@ -473,6 +473,32 @@
</span><span class="cx" style="display: block; padding: 0 10px">
</span><span class="cx" style="display: block; padding: 0 10px"> /**
</span><span class="cx" style="display: block; padding: 0 10px"> * @group xprofilemeta
</span><ins style="background-color: #dfd; text-decoration:none; display:block; padding: 0 10px">+ * @group bp_xprofile_update_meta
+ * @ticket BP5919
+ */
+ public function test_bp_xprofile_update_meta_where_sql_filter_keywords_are_in_quoted_value() {
+ $g = $this->factory->xprofile_group->create();
+ $value = "SELECT object_id FROM wp_bp_xprofile_groups WHERE \"foo\" VALUES (foo = 'bar'";
+ bp_xprofile_add_meta( $g, 'group', 'foo', 'bar' );
+ bp_xprofile_update_meta( $g, 'group', 'foo', $value );
+ $this->assertSame( $value, bp_xprofile_get_meta( $g, 'group', 'foo' ) );
+ }
+
+ /**
+ * @group xprofilemeta
+ * @group bp_xprofile_update_meta
+ * @ticket BP5919
+ */
+ public function test_bp_xprofile_update_meta_where_meta_id_is_in_quoted_value() {
+ $g = $this->factory->xprofile_group->create();
+ $value = "foo meta_id bar";
+ bp_xprofile_add_meta( $g, 'group', 'foo', 'bar' );
+ bp_xprofile_update_meta( $g, 'group', 'foo', $value );
+ $this->assertSame( $value, bp_xprofile_get_meta( $g, 'group', 'foo' ) );
+ }
+
+ /**
+ * @group xprofilemeta
</ins><span class="cx" style="display: block; padding: 0 10px"> * @group bp_xprofile_add_meta
</span><span class="cx" style="display: block; padding: 0 10px"> */
</span><span class="cx" style="display: block; padding: 0 10px"> public function test_bp_xprofile_add_meta_no_meta_key() {
</span></span></pre>
</div>
</div>
</body>
</html>