Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15384685
D8315.id19767.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
9 KB
Referenced Files
None
Subscribers
None
D8315.id19767.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -23,6 +23,7 @@
'AphrontMySQLDatabaseConnectionBase' => 'aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php',
'AphrontMySQLiDatabaseConnection' => 'aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php',
'AphrontQueryAccessDeniedException' => 'aphront/storage/exception/AphrontQueryAccessDeniedException.php',
+ 'AphrontQueryCharacterSetException' => 'aphront/storage/exception/AphrontQueryCharacterSetException.php',
'AphrontQueryConnectionException' => 'aphront/storage/exception/AphrontQueryConnectionException.php',
'AphrontQueryConnectionLostException' => 'aphront/storage/exception/AphrontQueryConnectionLostException.php',
'AphrontQueryCountException' => 'aphront/storage/exception/AphrontQueryCountException.php',
@@ -442,6 +443,7 @@
'AphrontMySQLDatabaseConnectionBase' => 'AphrontDatabaseConnection',
'AphrontMySQLiDatabaseConnection' => 'AphrontMySQLDatabaseConnectionBase',
'AphrontQueryAccessDeniedException' => 'AphrontQueryRecoverableException',
+ 'AphrontQueryCharacterSetException' => 'AphrontQueryException',
'AphrontQueryConnectionException' => 'AphrontQueryException',
'AphrontQueryConnectionLostException' => 'AphrontQueryRecoverableException',
'AphrontQueryCountException' => 'AphrontQueryException',
diff --git a/src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php b/src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
--- a/src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
+++ b/src/aphront/storage/connection/AphrontIsolatedDatabaseConnection.php
@@ -26,10 +26,14 @@
return;
}
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
return '<S>';
}
+ public function escapeBinaryString($string) {
+ return '<B>';
+ }
+
public function escapeColumnName($name) {
return '<C>';
}
diff --git a/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php b/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
--- a/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
+++ b/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnection.php
@@ -6,7 +6,12 @@
final class AphrontMySQLDatabaseConnection
extends AphrontMySQLDatabaseConnectionBase {
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
+ $this->validateUTF8String($string);
+ return $this->escapeBinaryString($string);
+ }
+
+ public function escapeBinaryString($string) {
return mysql_real_escape_string($string, $this->requireConnection());
}
diff --git a/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php b/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
--- a/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
+++ b/src/aphront/storage/connection/mysql/AphrontMySQLDatabaseConnectionBase.php
@@ -74,7 +74,7 @@
public function escapeStringForLikeClause($value) {
$value = addcslashes($value, '\%_');
- $value = $this->escapeString($value);
+ $value = $this->escapeUTF8String($value);
return $value;
}
@@ -321,4 +321,22 @@
return $this;
}
+ /**
+ * Check inserts for characters outside of the BMP. Even with the strictest
+ * settings, MySQL will silently truncate data when it encounters these, which
+ * can lead to data loss and security problems.
+ */
+ protected function validateUTF8String($string) {
+ if (phutil_is_utf8_with_only_bmp_characters($string)) {
+ return;
+ }
+
+ throw new AphrontQueryCharacterSetException(
+ pht(
+ 'Attempting to construct a query containing characters outside of '.
+ 'the Unicode Basic Multilingual Plane. MySQL will silently truncate '.
+ 'this data if it is inserted into a `utf8` column. Use the `%%B` '.
+ 'conversion to escape binary strings data.'));
+ }
+
}
diff --git a/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php b/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
--- a/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
+++ b/src/aphront/storage/connection/mysql/AphrontMySQLiDatabaseConnection.php
@@ -8,7 +8,12 @@
final class AphrontMySQLiDatabaseConnection
extends AphrontMySQLDatabaseConnectionBase {
- public function escapeString($string) {
+ public function escapeUTF8String($string) {
+ $this->validateUTF8String($string);
+ return $this->escapeBinaryString($string);
+ }
+
+ public function escapeBinaryString($string) {
return $this->requireConnection()->escape_string($string);
}
diff --git a/src/aphront/storage/exception/AphrontQueryCharacterSetException.php b/src/aphront/storage/exception/AphrontQueryCharacterSetException.php
new file mode 100644
--- /dev/null
+++ b/src/aphront/storage/exception/AphrontQueryCharacterSetException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class AphrontQueryCharacterSetException extends AphrontQueryException {
+
+}
diff --git a/src/xsprintf/PhutilQsprintfInterface.php b/src/xsprintf/PhutilQsprintfInterface.php
--- a/src/xsprintf/PhutilQsprintfInterface.php
+++ b/src/xsprintf/PhutilQsprintfInterface.php
@@ -4,7 +4,8 @@
* @group storage
*/
interface PhutilQsprintfInterface {
- public function escapeString($string);
+ public function escapeBinaryString($string);
+ public function escapeUTF8String($string);
public function escapeColumnName($string);
public function escapeMultilineComment($string);
public function escapeStringForLikeClause($string);
diff --git a/src/xsprintf/qsprintf.php b/src/xsprintf/qsprintf.php
--- a/src/xsprintf/qsprintf.php
+++ b/src/xsprintf/qsprintf.php
@@ -5,8 +5,8 @@
* all the normal conversions (like %s) will be properly escaped, and
* additional conversions are supported:
*
- * %nd, %ns, %nf
- * "Nullable" versions of %d, %s and %f. Will produce 'NULL' if the
+ * %nd, %ns, %nf, %nB
+ * "Nullable" versions of %d, %s, %f and %B. Will produce 'NULL' if the
* argument is a strict null.
*
* %=d, %=s, %=f
@@ -16,12 +16,16 @@
*
* qsprintf($escaper, 'WHERE hatID %=d', $hat);
*
- * %Ld, %Ls, %Lf
- * "List" versions of %d, %s and %f. These are appropriate for use in
+ * %Ld, %Ls, %Lf, %LB
+ * "List" versions of %d, %s, %f and %B. These are appropriate for use in
* an "IN" clause. For example:
*
* qsprintf($escaper, 'WHERE hatID IN(%Ld)', $list_of_hats);
*
+ * %B ("Binary String")
+ * Escapes a string for insertion into a pure binary column, ignoring
+ * tests for characters outside of the basic multilingual plane.
+ *
* %T ("Table")
* Escapes a table name.
*
@@ -116,6 +120,7 @@
case 'd': // ...integer.
case 'f': // ...float.
case 's': // ...string.
+ case 'B': // ...binary string.
$pattern = substr_replace($pattern, '', $pos, 1);
$length = strlen($pattern);
$type = $next;
@@ -139,7 +144,13 @@
break;
case 's': // ...strings.
foreach ($value as $k => $v) {
- $value[$k] = "'".$escaper->escapeString($v)."'";
+ $value[$k] = "'".$escaper->escapeUTF8String((string)$v)."'";
+ }
+ $value = implode(', ', $value);
+ break;
+ case 'B': // ...binary strings.
+ foreach ($value as $k => $v) {
+ $value[$k] = "'".$escaper->escapeBinaryString((string)$v)."'";
}
$value = implode(', ', $value);
break;
@@ -162,7 +173,16 @@
if ($nullable && $value === null) {
$value = 'NULL';
} else {
- $value = "'".$escaper->escapeString($value)."'";
+ $value = "'".$escaper->escapeUTF8String((string)$value)."'";
+ }
+ $type = 's';
+ break;
+
+ case 'B': // Binary String
+ if ($nullable && $value === null) {
+ $value = 'NULL';
+ } else {
+ $value = "'".$escaper->escapeBinaryString((string)$value)."'";
}
$type = 's';
break;
@@ -230,7 +250,7 @@
*/
function _qsprintf_check_type($value, $type, $query) {
switch ($type) {
- case 'Ld': case 'Ls': case 'LC': case 'LA': case 'LO':
+ case 'Ld': case 'Ls': case 'LC': case 'LB':
if (!is_array($value)) {
throw new AphrontQueryParameterException(
$query,
@@ -274,7 +294,7 @@
}
break;
- case 'Ls': case 's':
+ case 'Ls': case 's': case 'LB': case 'B':
case '~': case '>': case '<': case 'K':
if (!is_null($value) && !is_scalar($value)) {
throw new AphrontQueryParameterException(
@@ -283,15 +303,6 @@
}
break;
- case 'LA': case 'LO':
- if (!is_null($value) && !is_scalar($value) &&
- !(is_array($value) && !empty($value))) {
- throw new AphrontQueryParameterException(
- $query,
- "Expected a scalar or null or non-empty array for ".
- "%{$type} conversion.");
- }
- break;
default:
throw new Exception("Unknown conversion '{$type}'.");
}
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Mar 15, 8:34 PM (1 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7703418
Default Alt Text
D8315.id19767.diff (9 KB)
Attached To
Mode
D8315: Change "%s" in qsprintf to exclude non-BMP characters
Attached
Detach File
Event Timeline
Log In to Comment