Query: substitute bound values in a single pass

src/Neuron/DB/Query.php

@@ -333,18 +333,26 @@ public function getParsedQuery ()
 			}
 		}
 
-		// First we make a list with placeholders which we will later repalce with values
+		// Put a marker where each value goes, then swap all markers for
+		// their values in ONE pass. strtr() never looks at text it has
+		// already substituted, so nothing inside a value (not a '?', not a
+		// ':name', not marker-like text) can be taken for a placeholder.
+		// The markers also carry a random part, so a value cannot contain
+		// one in the first place.
+		$nonce = bin2hex (random_bytes (8));
+
 		$fakeValues = array ();
+		$replacements = array ();
 		foreach ($values as $k => $v) {
-			$fakeValues[$k] = '{{{ctlb-custom-placeholder-' . $k . '}}}';
+			$fakeValues[$k] = '{{{ctlb-' . $nonce . '-placeholder-' . $k . '}}}';
+			$replacements[$fakeValues[$k]] = (string) $v;
 		}
 
 		// And replace
 		$query = preg_replace ($keys, $fakeValues, $this->query, 1);
 
-		// And now replace the tokens with the actual values
-		foreach ($values as $k => $v) {
-			$query = str_replace ($fakeValues[$k], $v, $query);
+		if (count ($replacements) > 0) {
+			$query = strtr ($query, $replacements);
 		}
 
 		return $query;

tests/DbQueryInjectionTest.php

@@ -859,4 +859,54 @@ public function testMultipleInjectionVectorsInSingleQuery ()
 // All dangerous single quotes must be escaped
 $this->assertGreaterThanOrEqual (3, substr_count ($sql, "\\'"));
 }
+
+// ---------------------------------------------------------------
+// Values are substituted in one pass
+// ---------------------------------------------------------------
+
+public function testValueContainingOldMarkerOfLaterValueStaysLiteral ()
+{
+$query = new Query ("SELECT * FROM t WHERE a = ? AND b = ?");
+$query->bindValue (1, 'x {{{ctlb-custom-placeholder-1}}} y', Query::PARAM_STR);
+$query->bindValue (2, 'second', Query::PARAM_STR);
+$sql = $query->getParsedQuery ();
+
+$this->assertSame (
+"SELECT * FROM t WHERE a = 'x {{{ctlb-custom-placeholder-1}}} y' AND b = 'second'",
+$sql
+);
+}
+
+public function testBuilderValueContainingOldMarkerStaysLiteral ()
+{
+$sql = Query::select ('t', array (), array (
+'a' => array ('{{{ctlb-custom-placeholder-1}}}', Query::PARAM_STR),
+'b' => array ('second', Query::PARAM_STR),
+))->getParsedQuery ();
+
+$this->assertSame (
+"SELECT * FROM `t` WHERE a = '{{{ctlb-custom-placeholder-1}}}' AND b = 'second'",
+$sql
+);
+}
+
+public function testLaterValueIsNeverSubstitutedIntoEarlierValue ()
+{
+$query = new Query ("SELECT * FROM t WHERE a = :a AND b = :b");
+$query->bindValue ('a', '{{{ctlb-custom-placeholder-b}}}', Query::PARAM_STR);
+$query->bindValue ('b', 'second', Query::PARAM_STR);
+$sql = $query->getParsedQuery ();
+
+$this->assertSame (1, substr_count ($sql, "'second'"));
+$this->assertStringContainsString ("a = '{{{ctlb-custom-placeholder-b}}}'", $sql);
+}
+
+public function testMarkersDifferBetweenQueries ()
+{
+$reflection = new \ReflectionClass (Query::class);
+$source = file_get_contents ($reflection->getFileName ());
+
+$this->assertStringContainsString ('random_bytes', $source);
+$this->assertStringNotContainsString ("'{{{ctlb-custom-placeholder-'", $source);
+}
 }