Merge pull request #27 from mortenson/mortenson/fixes

Fix issues with closures, traits, aliases, and interfaces
Corveda · Jul 3, 2022 · 5495ff5 · 5495ff5
2 parents 3e2ed87 + 5a8fffa
commit 5495ff5
Show file tree

Hide file tree

Showing 9 changed files with 107 additions and 12 deletions.
diff --git a/src/PHPSandbox.php b/src/PHPSandbox.php
@@ -5946,13 +5946,13 @@ public function deblacklistType($name) : self {
         }
 
         /** Check function name against PHPSandbox validation rules. This is an internal PHPSandbox function but requires public access to work.
-         * @param   string      $name      String of the function name to check
+         * @param   string|Closure|SandboxedString      $name      String of the function name to check
          *
-         * @throws  Throwable   Throws exception if validation error occurs
+         * @throws  Throwable                           Throws exception if validation error occurs
          *
-         * @return  bool        Returns true if function is valid, this is also used for testing closures
+         * @return  bool                                Returns true if function is valid, this is also used for testing closures
          */
-        public function checkFunc(string $name) : bool {
+        public function checkFunc($name) : bool {
             if(!$this->validate_functions){
                 return true;
             }
@@ -6612,7 +6612,7 @@ protected function prepareAliases() : string {
             $output = [];
             foreach($this->definitions['aliases'] as $alias){
                 if(is_array($alias) && isset($alias['original']) && is_string($alias['original']) && $alias['original']){
-                    $output[] = 'use ' . $alias['original'] . ((isset($alias['alias']) && is_string($alias['alias']) && $alias['alias']) ? ' as ' . $alias['alias'] : '') . ';';
+                    $output[] = 'use ' . $alias['original'] . ((isset($alias['alias']) && (is_string($alias['alias']) || $alias['alias'] instanceof Node\Identifier) && $alias['alias']) ? ' as ' . $alias['alias'] : '') . ';';
                 } else {
                     $this->validationError("Sandboxed code attempted to use invalid namespace alias: " . $alias['original'], Error::DEFINE_ALIAS_ERROR, null, $alias['original']);
                 }

diff --git a/src/SandboxWhitelistVisitor.php b/src/SandboxWhitelistVisitor.php
@@ -49,14 +49,14 @@ public function leaveNode(Node $node){
                 $this->sandbox->whitelistClass($node->name->toString());
                 $this->sandbox->whitelistType($node->name->toString());
             } else if($node instanceof Node\Stmt\Interface_
-                && is_string($node->name)
+                && (is_string($node->name) || $node->name instanceof Node\Identifier)
                 && $this->sandbox->allow_interfaces
                 && $this->sandbox->auto_whitelist_interfaces
                 && !$this->sandbox->hasBlacklistedInterfaces()
             ){
                 $this->sandbox->whitelistInterface($node->name);
             } else if($node instanceof Node\Stmt\Trait_
-                && is_string($node->name)
+                && (is_string($node->name) || $node->name instanceof Node\Identifier)
                 && $this->sandbox->allow_traits
                 && $this->sandbox->auto_whitelist_traits
                 && !$this->sandbox->hasBlacklistedTraits()

diff --git a/src/ValidatorVisitor.php b/src/ValidatorVisitor.php
@@ -5,6 +5,7 @@
     namespace PHPSandbox;
 
     use PhpParser\Node,
+        PhpParser\NodeTraverser,
         PhpParser\NodeVisitorAbstract,
         Throwable;
 
@@ -348,19 +349,21 @@ public function leaveNode(Node $node){
                     /**
                      * @var Node\Stmt\UseUse    $use
                      */
-                    if($use instanceof Node\Stmt\UseUse && $use->name instanceof Node\Name && (is_string($use->alias) || is_null($use->alias))){
+                    if($use instanceof Node\Stmt\UseUse && $use->name instanceof Node\Name && (is_string($use->alias) || $use->alias instanceof Node\Identifier || is_null($use->alias))){
                         $this->sandbox->checkAlias($use->name->toString());
                         if($use->alias){
                             if(!$this->sandbox->checkKeyword('as')){
                                 $this->sandbox->validationError('Keyword failed custom validation!', Error::VALID_KEYWORD_ERROR, $node, 'as');
                             }
                         }
-                        $this->sandbox->defineAlias($use->name->toString(), $use->alias);
+                        $this->sandbox->whitelistClass($use->getAlias());
+                        $this->sandbox->whitelistType($use->getAlias());
+                        $this->sandbox->defineAlias($use->name->toString(), $use->getAlias());
                     } else {
                         $this->sandbox->validationError('Sandboxed code attempted use invalid namespace or alias!', Error::DEFINE_ALIAS_ERROR, $node);
                     }
                 }
-                return false;
+                return NodeTraverser::REMOVE_NODE;
             } else if($node instanceof Node\Expr\ShellExec){
                 if($this->sandbox->isDefinedFunc('shell_exec')){
                     $args = [

diff --git a/src/WhitelistVisitor.php b/src/WhitelistVisitor.php
@@ -5,6 +5,7 @@
     namespace PHPSandbox;
 
     use PhpParser\Node,
+        PhpParser\NodeTraverser,
         PhpParser\NodeVisitorAbstract,
         Throwable;
 
@@ -115,7 +116,7 @@ public function leaveNode(Node $node) : ?bool {
                         $this->sandbox->defineNamespace($name);
                     }
                 }
-                return false;
+                return NodeTraverser::REMOVE_NODE;
             } else if($node instanceof Node\Stmt\Use_){
                 foreach($node->uses as $use){
                     if($use instanceof Node\Stmt\UseUse
@@ -129,7 +130,7 @@ public function leaveNode(Node $node) : ?bool {
                         }
                     }
                 }
-                return false;
+                return NodeTraverser::REMOVE_NODE;
             }
             return null;
         }

diff --git a/tests/CodeSamplesTest.php b/tests/CodeSamplesTest.php
@@ -37,4 +37,41 @@ public function testPropertiesWithMagickConstants() : void {
             $result = $this->sandbox->execute(file_get_contents($path), false, $path);
             $this->assertEquals($path, $result);
         }
+
+        public function testTraits() : void {
+            $path = __DIR__ . '/samples/traits/index.php';
+            $this->sandbox->allow_traits = true;
+            $this->sandbox->allow_classes = true;
+            $this->sandbox->capture_output = true;
+            $result = $this->sandbox->execute(file_get_contents($path), false, $path);
+            $this->assertEquals('ok', $result);
+        }
+
+        public function testInterfaces() : void {
+            $path = __DIR__ . '/samples/interfaces/index.php';
+            $this->sandbox->allow_interfaces = true;
+            $this->sandbox->allow_classes = true;
+            $this->sandbox->capture_output = true;
+            $result = $this->sandbox->execute(file_get_contents($path), false, $path);
+            $this->assertEquals('ok', $result);
+        }
+
+        public function testClosures() : void {
+            $path = __DIR__ . '/samples/closures/index.php';
+            $this->sandbox->allow_closures = true;
+            $this->sandbox->allow_functions = true;
+            $this->sandbox->capture_output = true;
+            $result = $this->sandbox->execute(file_get_contents($path), false, $path);
+            $this->assertEquals('ok', $result);
+        }
+
+        public function testAliases() : void {
+            $path = __DIR__ . '/samples/aliases/index.php';
+            $this->sandbox->allow_aliases = true;
+            $this->sandbox->capture_output = true;
+            $this->sandbox->whitelistType('PHPUnit\Framework\Exception');
+            $this->sandbox->whitelistType('Exception');
+            $result = $this->sandbox->execute(file_get_contents($path), false, $path);
+            $this->assertEquals('okok', $result);
+        }
     }
diff --git a/tests/samples/aliases/index.php b/tests/samples/aliases/index.php
@@ -0,0 +1,11 @@
+<?php
+
+use PHPUnit\Framework\Exception;
+
+use PHPUnit\Runner\Exception as OtherException;
+
+$exception = new Exception('ok');
+echo $exception->getMessage();
+
+$exception = new OtherException('ok');
+echo $exception->getMessage();
diff --git a/tests/samples/closures/index.php b/tests/samples/closures/index.php
@@ -0,0 +1,7 @@
+<?php
+
+$func = function (){
+  echo "ok";
+};
+
+$func();
diff --git a/tests/samples/interfaces/index.php b/tests/samples/interfaces/index.php
@@ -0,0 +1,18 @@
+<?php
+
+interface MyInterface {
+
+  function getText();
+
+}
+
+class MyImplementor implements MyInterface {
+
+  function getText(){
+    echo 'ok';
+  }
+
+}
+
+$foo = new MyImplementor();
+echo $foo->getText();
diff --git a/tests/samples/traits/index.php b/tests/samples/traits/index.php
@@ -0,0 +1,18 @@
+<?php
+
+trait MyTrait {
+
+  function getText () {
+    return 'ok';
+  }
+
+}
+
+class MyClass {
+
+  use MyTrait;
+
+}
+
+$foo = new MyClass();
+echo $foo->getText();