Brian Wolff has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/395756 )
Change subject: Minor fixes to the eval case ...................................................................... Minor fixes to the eval case Include a doc comment about how setFuncTaint() can lead to misleading issues. Change-Id: I5344f1b57dfb5f450fdff68a51467608cf4c8d6a --- M src/TaintednessBaseVisitor.php M src/TaintednessVisitor.php 2 files changed, 7 insertions(+), 3 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/tools/phan/SecurityCheckPlugin refs/changes/56/395756/1 diff --git a/src/TaintednessBaseVisitor.php b/src/TaintednessBaseVisitor.php index 683f0e2..c87b47e 100644 --- a/src/TaintednessBaseVisitor.php +++ b/src/TaintednessBaseVisitor.php @@ -59,6 +59,10 @@ /** * Change taintedness of a function/method * + * @todo FIXME, the taintedOriginalError property should be on + * a per parameter granuality, instead of per function. As it + * stands this means very misleading error messages can be + * output. See the json integration test for an example. * @param FunctionInterface $func * @param int[] $taint Numeric keys for each arg and an 'overall' key. * @param bool $override Whether to merge taint or override diff --git a/src/TaintednessVisitor.php b/src/TaintednessVisitor.php index 159b318..09039ab 100644 --- a/src/TaintednessVisitor.php +++ b/src/TaintednessVisitor.php @@ -457,7 +457,7 @@ // in case it later turns out not to be safe. $phanObjs = $this->getPhanObjsForNode( $node->children['expr'] ); foreach ( $phanObjs as $phanObj ) { - $this->debug( __METHOD__, "Setting $phanObj exec due to echo" ); + $this->debug( __METHOD__, "Setting $phanObj exec due to backtick" ); $this->markAllDependentMethodsExec( $phanObj, SecurityCheckPlugin::SHELL_EXEC_TAINT @@ -483,14 +483,14 @@ ); if ( - $this->isSafeAssignment( SecurityCheckPlugin::HTML_EXEC_TAINT, $taintedness ) && + $this->isSafeAssignment( SecurityCheckPlugin::MISC_EXEC_TAINT, $taintedness ) && is_object( $node->children['expr'] ) ) { // In the event the assignment looks safe, keep track of it, // in case it later turns out not to be safe. $phanObjs = $this->getPhanObjsForNode( $node->children['expr'] ); foreach ( $phanObjs as $phanObj ) { - $this->debug( __METHOD__, "Setting $phanObj exec due to echo" ); + $this->debug( __METHOD__, "Setting $phanObj exec due to require/eval" ); $this->markAllDependentMethodsExec( $phanObj, SecurityCheckPlugin::MISC_EXEC_TAINT -- To view, visit https://gerrit.wikimedia.org/r/395756 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5344f1b57dfb5f450fdff68a51467608cf4c8d6a Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/tools/phan/SecurityCheckPlugin Gerrit-Branch: master Gerrit-Owner: Brian Wolff <bawolff...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits