diff --git a/src/Neuron/DB/Query.php b/src/Neuron/DB/Query.php index b7523fe..6d07ae3 100644 --- a/src/Neuron/DB/Query.php +++ b/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; diff --git a/tests/DbQueryInjectionTest.php b/tests/DbQueryInjectionTest.php index a4a82a7..7a8495e 100644 --- a/tests/DbQueryInjectionTest.php +++ b/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); +} }