From 023e899a874298fa67bcfcbce0568f2851c3196e Mon Sep 17 00:00:00 2001 From: Dmitry Khrysev Date: Thu, 14 Jan 2021 18:07:03 +0200 Subject: [PATCH] OIS-417: Upgrade phpstan to 0.12.* for SQL Injection testing (#6) * OIS-417: Upgrade phpstan to 0.12.* for SQL Injection testing --- composer.json | 6 +- extension.neon | 93 ++++ phpstan.neon | 8 + rules.neon | 484 ++++++++---------- .../DI/SqlInjectionTestingConfigExtension.php | 57 --- .../Methods/QueryBuilderInjectionRule.php | 12 + ...etRepositoryDynamicReturnTypeExtension.php | 5 +- src/Oro/TrustedDataConfigurationFinder.php | 10 +- 8 files changed, 331 insertions(+), 344 deletions(-) create mode 100644 extension.neon create mode 100644 phpstan.neon delete mode 100644 src/Oro/DI/SqlInjectionTestingConfigExtension.php diff --git a/composer.json b/composer.json index d48d275..5fb545d 100644 --- a/composer.json +++ b/composer.json @@ -19,8 +19,8 @@ "psr-4": {"": "src/"} }, "require": { - "phpstan/phpstan-shim": "0.11.*", - "phpstan/phpstan-doctrine": "0.11.*", - "ocramius/proxy-manager": "2.2.*" + "phpstan/phpstan": "0.12.*", + "phpstan/phpstan-doctrine": "0.12.*", + "nette/neon": "^3.1" } } diff --git a/extension.neon b/extension.neon new file mode 100644 index 0000000..2bc211c --- /dev/null +++ b/extension.neon @@ -0,0 +1,93 @@ +includes: + - ../../phpstan/phpstan-doctrine/extension.neon + +services: + - + class: Oro\Rules\Methods\QueryBuilderInjectionRule + arguments: + checkThisOnly: %checkThisOnly% + trustedData: %sql_injection_testing.trusted_data% + tags: + - phpstan.rules.rule + + - + class: Oro\Rules\Types\RequestGetSessionTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Oro\Rules\Types\EntityManagerReturnTypeExtension + arguments: + supportedClass: Doctrine\ORM\EntityManagerInterface + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Oro\Rules\Types\EntityManagerReturnTypeExtension + arguments: + supportedClass: Doctrine\Persistence\ManagerRegistry + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Oro\Rules\Types\ManagerRegistryEMReturnTypeExtension + arguments: + supportedClass: Doctrine\Persistence\ManagerRegistry + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + - + class: Oro\Rules\Types\ObjectRepositoryReturnTypeExtension + arguments: + supportedClass: Doctrine\ORM\EntityRepository + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + + persistenceManagerRegistryGetRepository: + class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + arguments: + managerClass: Doctrine\Persistence\ManagerRegistry + + persistenceObjectManagerGetRepository: + class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + arguments: + managerClass: Doctrine\Persistence\ObjectManager + + managerRegistryGetRepository: + class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + arguments: + managerClass: Doctrine\Persistence\ManagerRegistry + + objectManagerGetRepository: + class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension + tags: + - phpstan.broker.dynamicMethodReturnTypeExtension + arguments: + managerClass: Doctrine\Persistence\ObjectManager + + # Should be uncommented after existing problems fixed + #- + #class: Oro\Rules\ValidExceptionCatchRule + #tags: + #- phpstan.rules.rule + +parametersSchema: + sql_injection_testing: structure([ + trusted_data: structure([ + variables: arrayOf(arrayOf(arrayOf(bool()))) + properties: arrayOf(arrayOf(arrayOf(bool()))) + safe_static_methods: arrayOf(arrayOf(bool())) + safe_methods: arrayOf(arrayOf(anyOf(bool(), arrayOf(int())))) + check_static_methods_safety: arrayOf(arrayOf(anyOf(bool(), arrayOf(int())))) + check_methods_safety: arrayOf(arrayOf(anyOf(bool(), arrayOf(int())))) + check_methods: arrayOf(arrayOf(anyOf(bool(), arrayOf(int())))) + clear_static_methods: arrayOf(arrayOf(bool())) + clear_methods: arrayOf(arrayOf(bool())) + ]) + ]) diff --git a/phpstan.neon b/phpstan.neon new file mode 100644 index 0000000..63cecbd --- /dev/null +++ b/phpstan.neon @@ -0,0 +1,8 @@ +includes: + - extension.neon + - rules.neon + +parameters: + customRulesetUsed: true + checkThisOnly: false + inferPrivatePropertyTypeFromConstructor: true diff --git a/rules.neon b/rules.neon index e0800be..b2a19ff 100644 --- a/rules.neon +++ b/rules.neon @@ -1,279 +1,207 @@ -includes: - - ../../phpstan/phpstan-doctrine/extension.neon - -extensions: - sql_injection_testing: Oro\DI\SqlInjectionTestingConfigExtension - parameters: - customRulesetUsed: true - checkThisOnly: false - inferPrivatePropertyTypeFromConstructor: true - -services: - - - class: Oro\Rules\Methods\QueryBuilderInjectionRule - arguments: - checkThisOnly: %checkThisOnly% - trustedData: %trusted_data% - tags: - - phpstan.rules.rule - - - - class: Oro\Rules\Types\RequestGetSessionTypeExtension - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - - - - class: Oro\Rules\Types\EntityManagerReturnTypeExtension - arguments: - supportedClass: Doctrine\ORM\EntityManagerInterface - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - - - - class: Oro\Rules\Types\EntityManagerReturnTypeExtension - arguments: - supportedClass: Doctrine\Common\Persistence\ManagerRegistry - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - - - - class: Oro\Rules\Types\ManagerRegistryEMReturnTypeExtension - arguments: - supportedClass: Doctrine\Common\Persistence\ManagerRegistry - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - - - - class: Oro\Rules\Types\ObjectRepositoryReturnTypeExtension - arguments: - supportedClass: Doctrine\ORM\EntityRepository - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - - managerRegistryGetRepository: - class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - arguments: - managerClass: Doctrine\Common\Persistence\ManagerRegistry - - objectManagerGetRepository: - class: Oro\Rules\Types\GetRepositoryDynamicReturnTypeExtension - tags: - - phpstan.broker.dynamicMethodReturnTypeExtension - arguments: - managerClass: Doctrine\Common\Persistence\ObjectManager - - # Should be uncommented after existing problems fixed - #- - #class: Oro\Rules\ValidExceptionCatchRule - #tags: - #- phpstan.rules.rule - -sql_injection_testing: - trusted_data: - variables: [] - properties: [] - safe_static_methods: [] - - safe_methods: - Doctrine\DBAL\Connection: - quote: true - quoteIdentifier: true - - Doctrine\DBAL\Driver\Connection: - quote: true - - Doctrine\ORM\Query\Expr\Join: - getJoin: true - getAlias: true - getConditionType: true - getIndexBy: true - getCondition: true - - Doctrine\ORM\QueryBuilder: - setParameter: true - setParameters: true - setMaxResults: true - setFirstResult: true - addCriteria: true - getAllAliases: true - getRootAliases: true - getRootAlias: true - getRootEntities: true - getMaxResults: true - getFirstResult: true - getParameter: true - getDQL: true - getQuery: true - - Doctrine\ORM\Query: - getDQL: true - getSQL: true - - Doctrine\DBAL\Query\QueryBuilder: - setParameter: true - setParameters: true - setMaxResults: true - setFirstResult: true - getAllAliases: true - getRootAliases: true - getRootAlias: true - getRootEntities: true - getMaxResults: true - getFirstResult: true - getParameter: true - getQuery: true - - Doctrine\ORM\Mapping\ClassMetadata: - getSingleIdentifierFieldName: true - getIdentifierFieldNames: true - getIdentifier: true - getTableName: true - getAssociationMapping: true - - Doctrine\ORM\Mapping\ClassMetadataInfo: - getSingleIdentifierFieldName: true - getIdentifierFieldNames: true - getIdentifier: true - getTableName: true - getAssociationMapping: true - - Doctrine\Common\Persistence\Mapping\ClassMetadata: - getIdentifier: true - getIdentifierFieldNames: true - getIdentifierValues: true - - Doctrine\ORM\Query\Expr: - literal: true - - Doctrine\ORM\Query\Expr\Base: - add: true - - Doctrine\Common\Collections\ArrayCollection: - count: true - - DateTime: - format: true - - Doctrine\Common\Collections\Criteria: - getMaxResults: true - getFirstResult: true - - Symfony\Component\Validator\Context\ExecutionContextInterface: - getPropertyName: true - getClassName: true - - check_static_methods_safety: [] - - check_methods_safety: - Doctrine\Common\Inflector\Inflector: - pluralize: true - camelize: true - - Doctrine\Common\Collections\ExpressionBuilder: - notExists: [0] - - check_methods: - Doctrine\ORM\QueryBuilder: - __all__: true - where: [0, 1] - orWhere: [0, 1] - andWhere: [0, 1] - having: [0, 1] - orHaving: [0, 1] - andHaving: [0, 1] - join: [0, 1, 3] - leftJoin: [0, 1, 3] - innerJoin: [0, 1, 3] - - Doctrine\DBAL\Query\QueryBuilder: - __all__: true - where: [0, 1] - orWhere: [0, 1] - andWhere: [0, 1] - having: [0, 1] - orHaving: [0, 1] - andHaving: [0, 1] - join: [0, 1, 3] - leftJoin: [0, 1, 3] - innerJoin: [0, 1, 3] - - Doctrine\ORM\Query\Expr: - __all__: true - in: [0] - notIn: [0] - eq: [0, 1] - neq: [0, 1] - gt: [0, 1] - lt: [0, 1] - gte: [0, 1] - lte: [0, 1] - like: [0, 1] - notLike: [0, 1] - between: [1, 2] - isMemberOf: [0, 1] - - Doctrine\ORM\Query\Expr\Andx: - add: true - addMultiple: true - - Doctrine\ORM\Query\Expr\Orx: - add: true - addMultiple: true - - Doctrine\ORM\Query\Expr\Select: - add: true - addMultiple: true - - Doctrine\ORM\Query\Expr\GroupBy: - add: true - addMultiple: true - - Doctrine\ORM\Query\Expr\Coalesce: - add: true - addMultiple: true - - Doctrine\DBAL\Query\Expression\ExpressionBuilder: - __all__: true - - Doctrine\DBAL\Connection: - fetchAssoc: [0] - fetchArray: [0] - fetchColumn: [0] - fetchAll: [0] - delete: [0, 1] - update: [0, 1, 2] - insert: [0, 1] - prepare: true - executeQuery: [0] - executeCacheQuery: [0] - project: [0] - query: [0, 1] - executeUpdate: [0] - exec: true - - Doctrine\ORM\EntityManager: - createQuery: true - createNativeQuery: true - - clear_static_methods: [] - - clear_methods: - Doctrine\ORM\Mapping\ClassMetadata: - getAssociationTargetClass: true - getAssociationMapping: true - getFieldMapping: true - getSingleAssociationJoinColumnName: true - getSingleAssociationReferencedJoinColumnName: true - getFieldForColumn: true - - Doctrine\Common\Persistence\ManagerRegistry: - getManagerForClass: true - - Doctrine\Bundle\DoctrineBundle\Registry: - getManagerForClass: true - - Symfony\Bridge\Doctrine\RegistryInterface: - getManagerForClass: true + sql_injection_testing: + trusted_data: + variables: [] + properties: [] + safe_static_methods: [] + + safe_methods: + Doctrine\DBAL\Connection: + quote: true + quoteIdentifier: true + + Doctrine\DBAL\Driver\Connection: + quote: true + + Doctrine\ORM\Query\Expr\Join: + getJoin: true + getAlias: true + getConditionType: true + getIndexBy: true + getCondition: true + + Doctrine\ORM\QueryBuilder: + setParameter: true + setParameters: true + setMaxResults: true + setFirstResult: true + addCriteria: true + getAllAliases: true + getRootAliases: true + getRootAlias: true + getRootEntities: true + getMaxResults: true + getFirstResult: true + getParameter: true + getDQL: true + getQuery: true + + Doctrine\ORM\Query: + getDQL: true + getSQL: true + + Doctrine\DBAL\Query\QueryBuilder: + setParameter: true + setParameters: true + setMaxResults: true + setFirstResult: true + getAllAliases: true + getRootAliases: true + getRootAlias: true + getRootEntities: true + getMaxResults: true + getFirstResult: true + getParameter: true + getQuery: true + + Doctrine\ORM\Mapping\ClassMetadata: + getSingleIdentifierFieldName: true + getIdentifierFieldNames: true + getIdentifier: true + getTableName: true + getAssociationMapping: true + + Doctrine\ORM\Mapping\ClassMetadataInfo: + getSingleIdentifierFieldName: true + getIdentifierFieldNames: true + getIdentifier: true + getTableName: true + getAssociationMapping: true + + Doctrine\Persistence\Mapping\ClassMetadata: + getIdentifier: true + getIdentifierFieldNames: true + getIdentifierValues: true + + Doctrine\ORM\Query\Expr: + literal: true + + Doctrine\ORM\Query\Expr\Base: + add: true + + Doctrine\Common\Collections\ArrayCollection: + count: true + + DateTime: + format: true + + Doctrine\Common\Collections\Criteria: + getMaxResults: true + getFirstResult: true + + Symfony\Component\Validator\Context\ExecutionContextInterface: + getPropertyName: true + getClassName: true + + check_static_methods_safety: [] + + check_methods_safety: + Doctrine\Common\Inflector\Inflector: + pluralize: true + camelize: true + + Doctrine\Common\Collections\ExpressionBuilder: + notExists: [0] + + check_methods: + Doctrine\ORM\QueryBuilder: + __all__: true + where: [0, 1] + orWhere: [0, 1] + andWhere: [0, 1] + having: [0, 1] + orHaving: [0, 1] + andHaving: [0, 1] + join: [0, 1, 3] + leftJoin: [0, 1, 3] + innerJoin: [0, 1, 3] + + Doctrine\DBAL\Query\QueryBuilder: + __all__: true + where: [0, 1] + orWhere: [0, 1] + andWhere: [0, 1] + having: [0, 1] + orHaving: [0, 1] + andHaving: [0, 1] + join: [0, 1, 3] + leftJoin: [0, 1, 3] + innerJoin: [0, 1, 3] + + Doctrine\ORM\Query\Expr: + __all__: true + in: [0] + notIn: [0] + eq: [0, 1] + neq: [0, 1] + gt: [0, 1] + lt: [0, 1] + gte: [0, 1] + lte: [0, 1] + like: [0, 1] + notLike: [0, 1] + between: [1, 2] + isMemberOf: [0, 1] + + Doctrine\ORM\Query\Expr\Andx: + add: true + addMultiple: true + + Doctrine\ORM\Query\Expr\Orx: + add: true + addMultiple: true + + Doctrine\ORM\Query\Expr\Select: + add: true + addMultiple: true + + Doctrine\ORM\Query\Expr\GroupBy: + add: true + addMultiple: true + + Doctrine\ORM\Query\Expr\Coalesce: + add: true + addMultiple: true + + Doctrine\DBAL\Query\Expression\ExpressionBuilder: + __all__: true + + Doctrine\DBAL\Connection: + fetchAssoc: [0] + fetchArray: [0] + fetchColumn: [0] + fetchAll: [0] + delete: [0, 1] + update: [0, 1, 2] + insert: [0, 1] + prepare: true + executeQuery: [0] + executeCacheQuery: [0] + project: [0] + query: [0, 1] + executeUpdate: [0] + exec: true + + Doctrine\ORM\EntityManager: + createQuery: true + createNativeQuery: true + + clear_static_methods: [] + + clear_methods: + Doctrine\ORM\Mapping\ClassMetadata: + getAssociationTargetClass: true + getAssociationMapping: true + getFieldMapping: true + getSingleAssociationJoinColumnName: true + getSingleAssociationReferencedJoinColumnName: true + getFieldForColumn: true + + Doctrine\Persistence\ManagerRegistry: + getManagerForClass: true + + Doctrine\Bundle\DoctrineBundle\Registry: + getManagerForClass: true + + Symfony\Bridge\Doctrine\RegistryInterface: + getManagerForClass: true diff --git a/src/Oro/DI/SqlInjectionTestingConfigExtension.php b/src/Oro/DI/SqlInjectionTestingConfigExtension.php deleted file mode 100644 index 5ace636..0000000 --- a/src/Oro/DI/SqlInjectionTestingConfigExtension.php +++ /dev/null @@ -1,57 +0,0 @@ -validateConfig($this->loadFromFile($file)); - } - - $this->getContainerBuilder()->parameters = Helpers::merge( - $this->getContainerBuilder()->parameters, - $this->config - ); - } -} diff --git a/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php b/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php index f53154f..f892ed3 100644 --- a/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php +++ b/src/Oro/Rules/Methods/QueryBuilderInjectionRule.php @@ -2,6 +2,7 @@ namespace Oro\Rules\Methods; +use Nette\Neon\Neon; use PhpParser\Node; use PHPStan\Analyser\Scope; use PHPStan\Rules\RuleLevelHelper; @@ -673,6 +674,17 @@ private function checkClearMethodCall($type, $className, Node $value, Scope $sco */ protected function loadTrustedData(array $loadedData) { + // Load trusted_data.neon files + $configs = [$loadedData]; + foreach (\Oro\TrustedDataConfigurationFinder::findFiles() as $file) { + $config = Neon::decode(file_get_contents($file)); + if (!array_key_exists('trusted_data', $config)) { + continue; + } + $configs[] = $config['trusted_data']; + } + $loadedData = array_merge_recursive(...$configs); + $data = []; $lowerVariables = function ($loaded, $key) use (&$data) { diff --git a/src/Oro/Rules/Types/GetRepositoryDynamicReturnTypeExtension.php b/src/Oro/Rules/Types/GetRepositoryDynamicReturnTypeExtension.php index d971e47..517f01f 100644 --- a/src/Oro/Rules/Types/GetRepositoryDynamicReturnTypeExtension.php +++ b/src/Oro/Rules/Types/GetRepositoryDynamicReturnTypeExtension.php @@ -6,6 +6,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Reflection\MethodReflection; use PHPStan\Type\Doctrine\GetRepositoryDynamicReturnTypeExtension as BasenameExtension; +use PHPStan\Type\Generic\GenericObjectType; use PHPStan\Type\MixedType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; @@ -21,7 +22,9 @@ public function getTypeFromMethodCall( Scope $scope ): Type { $type = parent::getTypeFromMethodCall($methodReflection, $methodCall, $scope); - if ($type instanceof MixedType) { + if ($type instanceof MixedType + || ($type instanceof GenericObjectType && $type->getClassName() === 'Doctrine\Persistence\ObjectRepository') + ) { return new ObjectType('Doctrine\ORM\EntityRepository'); } diff --git a/src/Oro/TrustedDataConfigurationFinder.php b/src/Oro/TrustedDataConfigurationFinder.php index b0129ff..dfebfa4 100644 --- a/src/Oro/TrustedDataConfigurationFinder.php +++ b/src/Oro/TrustedDataConfigurationFinder.php @@ -15,7 +15,7 @@ class TrustedDataConfigurationFinder /** * @return array */ - public static function findFiles() + public static function findFiles(): array { $directories = self::getAutoloadDirectories(); @@ -49,7 +49,7 @@ function ($dirs) use (&$directories) { $prefixesPsr0 = $loader->getPrefixes(); array_walk( $prefixesPsr0, - function ($dirs) use (&$directories) { + static function ($dirs) use (&$directories) { $directories[] = array_values($dirs); } ); @@ -74,13 +74,13 @@ function ($dirs) use (&$directories) { /** * @return ClassLoader[] */ - private static function getRegisteredComposerAutoloaders() + private static function getRegisteredComposerAutoloaders(): array { - $composerLoaderClasses = array_filter(get_declared_classes(), function ($className) { + $composerLoaderClasses = array_filter(get_declared_classes(), static function ($className) { return strpos($className, self::COMPOSER_AUTOLOADER_INIT) === 0; }); - return array_map(function ($className) { + return array_map(static function ($className) { return \call_user_func([$className, 'getLoader']); }, $composerLoaderClasses); }