From 73847a4b194b26fb6948b1d8050d243f9c4a39fc Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 4 Sep 2020 17:01:13 -0700
Subject: [PATCH] Fix a false negative in lint for "xsprintf()"-family
 functions

Summary:
Ref T13577. This lint rule correctly detects the error in `pht('x %s y')` but the narrow test for `n_STRING_SCALAR` prevents it from detecting the error in `pht('x %s y'.'z')`.

Make the test broader.

Test Plan:
  - Ran `arc lint` on `HTTPSFuture.php`, got a detection of the issue in T13577.
  - Added a failing test and made it pass.

Maniphest Tasks: T13577

Differential Revision: https://secure.phabricator.com/D21453
---
 .../rules/ArcanistFormattedStringXHPASTLinterRule.php      | 7 ++++++-
 .../__tests__/formatted-string/formatted-string.lint-test  | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php b/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php
index 333d7ab1..7b5ed3bb 100644
--- a/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php
+++ b/src/lint/linter/xhpast/rules/ArcanistFormattedStringXHPASTLinterRule.php
@@ -82,7 +82,12 @@ final class ArcanistFormattedStringXHPASTLinterRule
       }
 
       $format = $parameters->getChildByIndex($start);
-      if ($format->getTypeName() != 'n_STRING_SCALAR') {
+      if (!$format->isConstantString()) {
+
+        // TODO: When this parameter is not a constant string, the call may
+        // be unsafe. We should make some attempt to warn about this for
+        // "qsprintf()" and other security-sensitive functions.
+
         continue;
       }
 
diff --git a/src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test b/src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test
index 388d6d63..67ec7746 100644
--- a/src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test
+++ b/src/lint/linter/xhpast/rules/__tests__/formatted-string/formatted-string.lint-test
@@ -11,12 +11,17 @@ fprintf(null, 'x');
 queryfx(null, 'x', 'y');
 
 foobar(null, null, '%s');
+
+pht('x %s y');
+pht('x %s y'.'z');
 ~~~~~~~~~~
 error:3:1:XHP54:Formatted String
 error:7:1:XHP54:Formatted String
 error:8:1:XHP54:Formatted String
 error:11:1:XHP54:Formatted String
 error:13:1:XHP54:Formatted String
+error:15:1:XHP54:Formatted String
+error:16:1:XHP54:Formatted String
 ~~~~~~~~~~
 ~~~~~~~~~~
 {