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

Reply via email to