From d74662df7df72ad9ec238b78223acc0e7f65311f Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Wed, 28 Jan 2015 22:08:50 +0100 Subject: implement php code checker to detect usage of not allowed private APIs - including console command to check local code to be used by developers --- lib/private/app/codechecker.php | 115 +++++++++++++++++++++++++++++++++ lib/private/app/codecheckervisitor.php | 111 +++++++++++++++++++++++++++++++ lib/private/installer.php | 2 +- 3 files changed, 227 insertions(+), 1 deletion(-) create mode 100644 lib/private/app/codechecker.php create mode 100644 lib/private/app/codecheckervisitor.php (limited to 'lib/private') diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php new file mode 100644 index 00000000000..28816a8fdc5 --- /dev/null +++ b/lib/private/app/codechecker.php @@ -0,0 +1,115 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\App; + +use OC\Hooks\BasicEmitter; +use PhpParser\Lexer; +use PhpParser\Node; +use PhpParser\Node\Name; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitorAbstract; +use PhpParser\Parser; +use RecursiveCallbackFilterIterator; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; +use RegexIterator; +use SplFileInfo; + +class CodeChecker extends BasicEmitter { + + const CLASS_EXTENDS_NOT_ALLOWED = 1000; + const CLASS_IMPLEMENTS_NOT_ALLOWED = 1001; + const STATIC_CALL_NOT_ALLOWED = 1002; + const CLASS_CONST_FETCH_NOT_ALLOWED = 1003; + const CLASS_NEW_FETCH_NOT_ALLOWED = 1004; + + public function __construct() { + $this->parser = new Parser(new Lexer); + $this->blackListedClassNames = [ + // classes replaced by the public api + 'OC_API', + 'OC_App', + 'OC_AppConfig', + 'OC_Avatar', + 'OC_BackgroundJob', + 'OC_Config', + 'OC_DB', + 'OC_Files', + 'OC_Helper', + 'OC_Hook', + 'OC_Image', + 'OC_JSON', + 'OC_L10N', + 'OC_Log', + 'OC_Mail', + 'OC_Preferences', + 'OC_Request', + 'OC_Response', + 'OC_Template', + 'OC_User', + 'OC_Util', + ]; + } + + /** + * @param string $appId + * @return array + */ + public function analyse($appId) { + $appPath = \OC_App::getAppPath($appId); + if ($appPath === false) { + throw new \RuntimeException("No app with given id <$appId> known."); + } + + $errors = []; + + $excludes = array_map(function($item) use ($appPath) { + return $appPath . '/' . $item; + }, ['vendor', '3rdparty', '.git', 'l10n']); + + $iterator = new RecursiveDirectoryIterator($appPath, RecursiveDirectoryIterator::SKIP_DOTS); + $iterator = new RecursiveCallbackFilterIterator($iterator, function($item) use ($appPath, $excludes){ + /** @var SplFileInfo $item */ + foreach($excludes as $exclude) { + if (substr($item->getPath(), 0, strlen($exclude)) === $exclude) { + return false; + } + } + return true; + }); + $iterator = new RecursiveIteratorIterator($iterator); + $iterator = new RegexIterator($iterator, '/^.+\.php$/i'); + + foreach ($iterator as $file) { + /** @var SplFileInfo $file */ + $this->emit('CodeChecker', 'analyseFileBegin', [$file->getPathname()]); + $errors = array_merge($this->analyseFile($file), $errors); + $this->emit('CodeChecker', 'analyseFileFinished', [$errors]); + } + + return $errors; + } + + /** + * @param string $file + * @return array + */ + public function analyseFile($file) { + $code = file_get_contents($file); + $statements = $this->parser->parse($code); + + $visitor = new CodeCheckVisitor($this->blackListedClassNames); + $traverser = new NodeTraverser; + $traverser->addVisitor($visitor); + + $traverser->traverse($statements); + + return $visitor->errors; + } +} diff --git a/lib/private/app/codecheckervisitor.php b/lib/private/app/codecheckervisitor.php new file mode 100644 index 00000000000..939c905bcf6 --- /dev/null +++ b/lib/private/app/codecheckervisitor.php @@ -0,0 +1,111 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\App; + +use OC\Hooks\BasicEmitter; +use PhpParser\Lexer; +use PhpParser\Node; +use PhpParser\Node\Name; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitorAbstract; +use PhpParser\Parser; +use RecursiveCallbackFilterIterator; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; +use RegexIterator; +use SplFileInfo; + +class CodeCheckVisitor extends NodeVisitorAbstract { + + public function __construct($blackListedClassNames) { + $this->blackListedClassNames = array_map('strtolower', $blackListedClassNames); + } + + public $errors = []; + + public function enterNode(Node $node) { + if ($node instanceof Node\Stmt\Class_) { + if (!is_null($node->extends)) { + $this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node); + } + foreach ($node->implements as $implements) { + $this->checkBlackList($implements->toString(), CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED, $node); + } + } + if ($node instanceof Node\Expr\StaticCall) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::STATIC_CALL_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = $i::call(); + */ + } + } + } + if ($node instanceof Node\Expr\ClassConstFetch) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = $i::ADMIN_AUTH; + */ + } + } + } + if ($node instanceof Node\Expr\New_) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = new $i; + */ + } + } + } + } + + private function checkBlackList($name, $errorCode, Node $node) { + if (in_array(strtolower($name), $this->blackListedClassNames)) { + $this->errors[]= [ + 'disallowedToken' => $name, + 'errorCode' => $errorCode, + 'line' => $node->getLine(), + 'reason' => $this->buildReason($name, $errorCode) + ]; + } + } + + private function buildReason($name, $errorCode) { + static $errorMessages= [ + CodeChecker::CLASS_EXTENDS_NOT_ALLOWED => "used as base class", + CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED => "used as interface", + CodeChecker::STATIC_CALL_NOT_ALLOWED => "static method call on private class", + CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED => "used to fetch a const from", + CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated", + ]; + + if (isset($errorMessages[$errorCode])) { + return $errorMessages[$errorCode]; + } + + return "$name usage not allowed - error: $errorCode"; + } +} diff --git a/lib/private/installer.php b/lib/private/installer.php index b4fbe527b4f..e77504f4c12 100644 --- a/lib/private/installer.php +++ b/lib/private/installer.php @@ -511,7 +511,7 @@ class OC_Installer{ OC_Appconfig::setValue($app, 'ocsid', $info['ocsid']); } - //set remote/public handelers + //set remote/public handlers foreach($info['remote'] as $name=>$path) { OCP\CONFIG::setAppValue('core', 'remote_'.$name, $app.'/'.$path); } -- cgit v1.2.3 From 9ecb36e81f703c5e7aae36c046f441e03f27cbdb Mon Sep 17 00:00:00 2001 From: Thomas Müller Date: Fri, 30 Jan 2015 17:31:51 +0100 Subject: integrate code checker in the installer --- lib/private/app/codechecker.php | 23 +++++-- lib/private/app/codecheckervisitor.php | 111 --------------------------------- lib/private/app/codecheckvisitor.php | 111 +++++++++++++++++++++++++++++++++ lib/private/installer.php | 58 +++-------------- 4 files changed, 138 insertions(+), 165 deletions(-) delete mode 100644 lib/private/app/codecheckervisitor.php create mode 100644 lib/private/app/codecheckvisitor.php (limited to 'lib/private') diff --git a/lib/private/app/codechecker.php b/lib/private/app/codechecker.php index 28816a8fdc5..dbec53579a8 100644 --- a/lib/private/app/codechecker.php +++ b/lib/private/app/codechecker.php @@ -29,6 +29,12 @@ class CodeChecker extends BasicEmitter { const CLASS_CONST_FETCH_NOT_ALLOWED = 1003; const CLASS_NEW_FETCH_NOT_ALLOWED = 1004; + /** @var Parser */ + private $parser; + + /** @var string[] */ + private $blackListedClassNames; + public function __construct() { $this->parser = new Parser(new Lexer); $this->blackListedClassNames = [ @@ -67,14 +73,22 @@ class CodeChecker extends BasicEmitter { throw new \RuntimeException("No app with given id <$appId> known."); } + return $this->analyseFolder($appPath); + } + + /** + * @param string $folder + * @return array + */ + public function analyseFolder($folder) { $errors = []; - $excludes = array_map(function($item) use ($appPath) { - return $appPath . '/' . $item; + $excludes = array_map(function($item) use ($folder) { + return $folder . '/' . $item; }, ['vendor', '3rdparty', '.git', 'l10n']); - $iterator = new RecursiveDirectoryIterator($appPath, RecursiveDirectoryIterator::SKIP_DOTS); - $iterator = new RecursiveCallbackFilterIterator($iterator, function($item) use ($appPath, $excludes){ + $iterator = new RecursiveDirectoryIterator($folder, RecursiveDirectoryIterator::SKIP_DOTS); + $iterator = new RecursiveCallbackFilterIterator($iterator, function($item) use ($folder, $excludes){ /** @var SplFileInfo $item */ foreach($excludes as $exclude) { if (substr($item->getPath(), 0, strlen($exclude)) === $exclude) { @@ -96,6 +110,7 @@ class CodeChecker extends BasicEmitter { return $errors; } + /** * @param string $file * @return array diff --git a/lib/private/app/codecheckervisitor.php b/lib/private/app/codecheckervisitor.php deleted file mode 100644 index 939c905bcf6..00000000000 --- a/lib/private/app/codecheckervisitor.php +++ /dev/null @@ -1,111 +0,0 @@ - - * This file is licensed under the Affero General Public License version 3 or - * later. - * See the COPYING-README file. - */ - -namespace OC\App; - -use OC\Hooks\BasicEmitter; -use PhpParser\Lexer; -use PhpParser\Node; -use PhpParser\Node\Name; -use PhpParser\NodeTraverser; -use PhpParser\NodeVisitorAbstract; -use PhpParser\Parser; -use RecursiveCallbackFilterIterator; -use RecursiveDirectoryIterator; -use RecursiveIteratorIterator; -use RegexIterator; -use SplFileInfo; - -class CodeCheckVisitor extends NodeVisitorAbstract { - - public function __construct($blackListedClassNames) { - $this->blackListedClassNames = array_map('strtolower', $blackListedClassNames); - } - - public $errors = []; - - public function enterNode(Node $node) { - if ($node instanceof Node\Stmt\Class_) { - if (!is_null($node->extends)) { - $this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node); - } - foreach ($node->implements as $implements) { - $this->checkBlackList($implements->toString(), CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED, $node); - } - } - if ($node instanceof Node\Expr\StaticCall) { - if (!is_null($node->class)) { - if ($node->class instanceof Name) { - $this->checkBlackList($node->class->toString(), CodeChecker::STATIC_CALL_NOT_ALLOWED, $node); - } - if ($node->class instanceof Node\Expr\Variable) { - /** - * TODO: find a way to detect something like this: - * $c = "OC_API"; - * $n = $i::call(); - */ - } - } - } - if ($node instanceof Node\Expr\ClassConstFetch) { - if (!is_null($node->class)) { - if ($node->class instanceof Name) { - $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED, $node); - } - if ($node->class instanceof Node\Expr\Variable) { - /** - * TODO: find a way to detect something like this: - * $c = "OC_API"; - * $n = $i::ADMIN_AUTH; - */ - } - } - } - if ($node instanceof Node\Expr\New_) { - if (!is_null($node->class)) { - if ($node->class instanceof Name) { - $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED, $node); - } - if ($node->class instanceof Node\Expr\Variable) { - /** - * TODO: find a way to detect something like this: - * $c = "OC_API"; - * $n = new $i; - */ - } - } - } - } - - private function checkBlackList($name, $errorCode, Node $node) { - if (in_array(strtolower($name), $this->blackListedClassNames)) { - $this->errors[]= [ - 'disallowedToken' => $name, - 'errorCode' => $errorCode, - 'line' => $node->getLine(), - 'reason' => $this->buildReason($name, $errorCode) - ]; - } - } - - private function buildReason($name, $errorCode) { - static $errorMessages= [ - CodeChecker::CLASS_EXTENDS_NOT_ALLOWED => "used as base class", - CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED => "used as interface", - CodeChecker::STATIC_CALL_NOT_ALLOWED => "static method call on private class", - CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED => "used to fetch a const from", - CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated", - ]; - - if (isset($errorMessages[$errorCode])) { - return $errorMessages[$errorCode]; - } - - return "$name usage not allowed - error: $errorCode"; - } -} diff --git a/lib/private/app/codecheckvisitor.php b/lib/private/app/codecheckvisitor.php new file mode 100644 index 00000000000..939c905bcf6 --- /dev/null +++ b/lib/private/app/codecheckvisitor.php @@ -0,0 +1,111 @@ + + * This file is licensed under the Affero General Public License version 3 or + * later. + * See the COPYING-README file. + */ + +namespace OC\App; + +use OC\Hooks\BasicEmitter; +use PhpParser\Lexer; +use PhpParser\Node; +use PhpParser\Node\Name; +use PhpParser\NodeTraverser; +use PhpParser\NodeVisitorAbstract; +use PhpParser\Parser; +use RecursiveCallbackFilterIterator; +use RecursiveDirectoryIterator; +use RecursiveIteratorIterator; +use RegexIterator; +use SplFileInfo; + +class CodeCheckVisitor extends NodeVisitorAbstract { + + public function __construct($blackListedClassNames) { + $this->blackListedClassNames = array_map('strtolower', $blackListedClassNames); + } + + public $errors = []; + + public function enterNode(Node $node) { + if ($node instanceof Node\Stmt\Class_) { + if (!is_null($node->extends)) { + $this->checkBlackList($node->extends->toString(), CodeChecker::CLASS_EXTENDS_NOT_ALLOWED, $node); + } + foreach ($node->implements as $implements) { + $this->checkBlackList($implements->toString(), CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED, $node); + } + } + if ($node instanceof Node\Expr\StaticCall) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::STATIC_CALL_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = $i::call(); + */ + } + } + } + if ($node instanceof Node\Expr\ClassConstFetch) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = $i::ADMIN_AUTH; + */ + } + } + } + if ($node instanceof Node\Expr\New_) { + if (!is_null($node->class)) { + if ($node->class instanceof Name) { + $this->checkBlackList($node->class->toString(), CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED, $node); + } + if ($node->class instanceof Node\Expr\Variable) { + /** + * TODO: find a way to detect something like this: + * $c = "OC_API"; + * $n = new $i; + */ + } + } + } + } + + private function checkBlackList($name, $errorCode, Node $node) { + if (in_array(strtolower($name), $this->blackListedClassNames)) { + $this->errors[]= [ + 'disallowedToken' => $name, + 'errorCode' => $errorCode, + 'line' => $node->getLine(), + 'reason' => $this->buildReason($name, $errorCode) + ]; + } + } + + private function buildReason($name, $errorCode) { + static $errorMessages= [ + CodeChecker::CLASS_EXTENDS_NOT_ALLOWED => "used as base class", + CodeChecker::CLASS_IMPLEMENTS_NOT_ALLOWED => "used as interface", + CodeChecker::STATIC_CALL_NOT_ALLOWED => "static method call on private class", + CodeChecker::CLASS_CONST_FETCH_NOT_ALLOWED => "used to fetch a const from", + CodeChecker::CLASS_NEW_FETCH_NOT_ALLOWED => "is instanciated", + ]; + + if (isset($errorMessages[$errorCode])) { + return $errorMessages[$errorCode]; + } + + return "$name usage not allowed - error: $errorCode"; + } +} diff --git a/lib/private/installer.php b/lib/private/installer.php index e77504f4c12..e50b5cea452 100644 --- a/lib/private/installer.php +++ b/lib/private/installer.php @@ -308,7 +308,7 @@ class OC_Installer{ } $info=OC_App::getAppInfo($extractDir.'/appinfo/info.xml', true); // check the code for not allowed calls - if(!$isShipped && !OC_Installer::checkCode($info['id'], $extractDir)) { + if(!$isShipped && !OC_Installer::checkCode($extractDir)) { OC_Helper::rmdirr($extractDir); throw new \Exception($l->t("App can't be installed because of not allowed code in the App")); } @@ -529,58 +529,16 @@ class OC_Installer{ * @param string $folder the folder of the app to check * @return boolean true for app is o.k. and false for app is not o.k. */ - public static function checkCode($appname, $folder) { - $blacklist=array( - // classes replaced by the public api - 'OC_API::', - 'OC_App::', - 'OC_AppConfig::', - 'OC_Avatar', - 'OC_BackgroundJob::', - 'OC_Config::', - 'OC_DB::', - 'OC_Files::', - 'OC_Helper::', - 'OC_Hook::', - 'OC_Image::', - 'OC_JSON::', - 'OC_L10N::', - 'OC_Log::', - 'OC_Mail::', - 'OC_Request::', - 'OC_Response::', - 'OC_Template::', - 'OC_User::', - 'OC_Util::', - ); + public static function checkCode($folder) { // is the code checker enabled? - if(OC_Config::getValue('appcodechecker', false)) { - // check if grep is installed - $grep = \OC_Helper::findBinaryPath('grep'); - if (!$grep) { - OC_Log::write('core', - 'grep not installed. So checking the code of the app "'.$appname.'" was not possible', - OC_Log::ERROR); - return true; - } - - // iterate the bad patterns - foreach($blacklist as $bl) { - $cmd = 'grep --include \\*.php -ri '.escapeshellarg($bl).' '.$folder.''; - $result = exec($cmd); - // bad pattern found - if($result<>'') { - OC_Log::write('core', - 'App "'.$appname.'" is using a not allowed call "'.$bl.'". Installation refused.', - OC_Log::ERROR); - return false; - } - } - return true; - - }else{ + if(!OC_Config::getValue('appcodechecker', false)) { return true; } + + $codeChecker = new \OC\App\CodeChecker(); + $errors = $codeChecker->analyseFolder($folder); + + return empty($errors); } } -- cgit v1.2.3