Page MenuHomePhabricator
Files F15422022

D20250.id.diff
No OneTemporary
Actions

D20250.id.diff
View Options

diff --git a/src/xsprintf/PhutilQueryString.php b/src/xsprintf/PhutilQueryString.php
--- a/src/xsprintf/PhutilQueryString.php
+++ b/src/xsprintf/PhutilQueryString.php
@@ -4,6 +4,7 @@
private $maskedString;
private $unmaskedString;
+ private $isMasked;
public function __construct(PhutilQsprintfInterface $escaper, array $argv) {
// Immediately render the query into a static scalar value.
@@ -30,13 +31,63 @@
),
$argv);
- $unmasked_string = xsprintf(
- 'xsprintf_query',
- array(
- 'escaper' => $escaper,
- 'unmasked' => true,
- ),
- $argv);
+ // As an optimization, only render the unmasked string if it may differ
+ // from the masked string. The strings may differ if any argument is a
+ // "PhutilOpaqueEnvelope", or if any argument is a "PhutilQueryString"
+ // with different masked and unmasked values.
+
+ // We always render the masked string and only render the unmasked string
+ // if we believe we need it, so a mistake here will cause us to incorrectly
+ // execute a query with "********" in it, not incorrectly disclose
+ // sensitive query parameters.
+
+ $need_unmasked = false;
+ foreach ($argv as $arg) {
+ if ($arg instanceof PhutilQueryString) {
+ if ($arg->isMasked) {
+ $need_unmasked = true;
+ break;
+ }
+ continue;
+ }
+
+ if ($arg instanceof PhutilOpaqueEnvelope) {
+ $need_unmasked = true;
+ break;
+ }
+
+ if (is_array($arg)) {
+ foreach ($arg as $item) {
+ if ($item instanceof PhutilQueryString) {
+ if ($item->isMasked) {
+ $need_unmasked = true;
+ break;
+ }
+ continue;
+ }
+
+ // If we find an array item which is not a "PhutilQueryString", we
+ // know this argument can not require us to unmask the string, since
+ // no valid argument to "qsprintf(...)" may be a list with a mixture
+ // of query strings and other values.
+ break;
+ }
+ }
+ }
+
+ if ($need_unmasked) {
+ $unmasked_string = xsprintf(
+ 'xsprintf_query',
+ array(
+ 'escaper' => $escaper,
+ 'unmasked' => true,
+ ),
+ $argv);
+ $this->isMasked = true;
+ } else {
+ $unmasked_string = $masked_string;
+ $this->isMasked = false;
+ }
$this->maskedString = $masked_string;
$this->unmaskedString = $unmasked_string;

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 23, 4:44 AM (1 d, 16 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7244745
Default Alt Text
D20250.id.diff (2 KB)

Event Timeline

Phabricator · Built by Phacility