revi-archive/phorge-arcanist
1
0
Fork 0
mirror of https://we.phorge.it/source/arcanist.git synced 2024-11-22 14:52:40 +01:00
Code Issues Releases Wiki Activity

Lint dynamic string usage in parameters treated as safe

Browse source 
Summary:
This disallows code like this:

  $cmd = 'ls';
  execx($cmd);

But I guess it's not that big deal?

Test Plan: Linted whole Arcanist and Phabricator codebases, most parts looks fixable.

Reviewers: epriestley

CC: nh, aran, Korvin

Differential Revision: https://secure.phabricator.com/D4794
This commit is contained in:
vrana 2013-02-02 12:33:22 -08:00
parent 39cef6cb99
commit 03199df925
1 changed files with 70 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

71
src/lint/linter/ArcanistPhutilXHPASTLinter.php
View file

@ -7,6 +7,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
const LINT_PHT_WITH_DYNAMIC_STRING = 1;
const LINT_ARRAY_COMBINE = 2;
const LINT_UNSAFE_DYNAMIC_STRING = 4;
private $xhpastLinter;
@ -28,12 +29,16 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
return array(
self::LINT_PHT_WITH_DYNAMIC_STRING => 'Use of pht() on Dynamic String',
self::LINT_ARRAY_COMBINE => 'array_combine() Unreliable',
self::LINT_UNSAFE_DYNAMIC_STRING => 'Unsafe Usage of Dynamic String',
);
}
public function getLintSeverityMap() {
$warning = ArcanistLintSeverity::SEVERITY_WARNING;
return array(
self::LINT_ARRAY_COMBINE => ArcanistLintSeverity::SEVERITY_WARNING,
self::LINT_ARRAY_COMBINE => $warning,
self::LINT_UNSAFE_DYNAMIC_STRING => $warning,
);
}
@ -41,6 +46,10 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
return 'PHLXHP';
}
public function getCacheVersion() {
return 1;
}
public function willLintPaths(array $paths) {
$this->xhpastLinter->willLintPaths($paths);
}
@ -55,6 +64,7 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
$this->lintPHT($root);
$this->lintArrayCombine($root);
$this->lintUnsafeDynamicString($root);
}
@ -85,6 +95,65 @@ final class ArcanistPhutilXHPASTLinter extends ArcanistBaseXHPASTLinter {
}
private function lintUnsafeDynamicString($root) {
$safe = array(
'hsprintf' => 0,
'csprintf' => 0,
'vcsprintf' => 0,
'execx' => 0,
'exec_manual' => 0,
'phutil_passthru' => 0,
'qsprintf' => 1,
'vqsprintf' => 1,
'queryfx' => 1,
'vqueryfx' => 1,
'queryfx_all' => 1,
'vqueryfx_all' => 1,
'queryfx_one' => 1,
);
$calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
$this->lintUnsafeDynamicStringCall($calls, $safe);
$safe = array(
'execfuture' => 0,
);
$news = $root->selectDescendantsOfType('n_NEW');
$this->lintUnsafeDynamicStringCall($news, $safe);
}
private function lintUnsafeDynamicStringCall(
AASTNodeList $calls,
array $safe) {
foreach ($calls as $call) {
$name = $call->getChildByIndex(0)->getConcreteString();
$param = idx($safe, strtolower($name));
if ($param === null) {
continue;
}
$parameters = $call->getChildByIndex(1);
if (count($parameters->getChildren()) <= $param) {
continue;
}
$identifier = $parameters->getChildByIndex($param);
if (!$identifier->isConstantString()) {
$this->raiseLintAtNode(
$call,
self::LINT_UNSAFE_DYNAMIC_STRING,
"Parameter ".($param + 1)." of {$name}() should be a scalar string, ".
"otherwise it's not safe.");
}
}
}
private function lintArrayCombine($root) {
$function_calls = $root->selectDescendantsOfType('n_FUNCTION_CALL');
foreach ($function_calls as $call) {