diff --git a/README.md b/README.md index 050e77d..32e4bdf 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,46 @@ function calls. Only simple, unmodified pass-through arguments (a bare are tracked — values that are transformed, wrapped, or reassigned before being passed on are not. +### Cryptographic callees are never flagged + +Passing a sensitive value directly into a hashing or encryption function is +the *intended* usage — there is nothing to propagate. The rule ships with a +built-in allowlist of well-known cryptographic functions and methods that will +never trigger a propagation warning: + +```php +function hashPassword(#[\SensitiveParameter] string $password): string +{ + return password_hash($password, PASSWORD_BCRYPT); // ✅ not flagged +} + +class AuthService +{ + public function store(#[\SensitiveParameter] string $password): void + { + $hash = Hash::make($password); // ✅ not flagged (Laravel) + } +} +``` + +The built-in allowlist covers: + +- **PHP core** — `password_hash`, `password_verify`, `hash`, `hash_hmac`, + `hash_pbkdf2`, `hash_equals`, `crypt`, `md5`, `sha1` +- **OpenSSL** — `openssl_encrypt`, `openssl_decrypt`, `openssl_digest`, + `openssl_sign`, `openssl_verify` +- **Sodium** — `sodium_crypto_pwhash`, `sodium_crypto_pwhash_str`, + `sodium_crypto_pwhash_str_verify`, `sodium_crypto_secretbox`, + `sodium_crypto_secretbox_open`, `sodium_crypto_auth`, + `sodium_crypto_auth_verify`, `sodium_crypto_box`, `sodium_crypto_box_open`, + `sodium_crypto_sign`, and others +- **Laravel** — `Illuminate\Support\Facades\Hash::make/check/needsRehash` + and the concrete `BcryptHasher`, `ArgonHasher`, `Argon2IdHasher` variants +- **LdapRecord** — `LdapRecord\Auth\Guard::attempt` + +See [Configuring the cryptographic callee allowlist](#configuring-the-cryptographic-callee-allowlist) +for how to add your own entries. + ## Storing sensitive values safely Marking a parameter sensitive prevents it from leaking through stack traces, @@ -233,23 +273,57 @@ class AuthService { ## Advanced Configuration -To use custom sensitive keywords instead of the defaults, override the service: +### Configuring sensitive keywords + +To use custom sensitive keywords instead of the defaults, set +`sensitiveParameter.keywords` in your `phpstan.neon`: ```neon -includes: - - vendor/built-fast/phpstan-sensitive-parameter/extension.neon - -services: - # Override the default service with custom keywords - - - class: LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule - arguments: - - ['password', 'apikey', 'token', 'banking', 'medical'] # Your custom keywords - tags: - - phpstan.rules.rule +parameters: + sensitiveParameter: + keywords: + - password + - apikey + - token + - banking + - medical ``` -This completely replaces the default keyword list with your own. +Providing a non-empty list **completely replaces** the default keyword list. + +### Configuring the cryptographic callee allowlist + +If your project uses a custom hashing or encryption wrapper that should not +trigger a propagation warning, add it to `sensitiveParameter.cryptoCallees`: + +```neon +parameters: + sensitiveParameter: + cryptoCallees: + - 'App\Security\Hasher::hash' + - 'App\Security\Hasher::verify' +``` + +Entries are matched as: +- **Plain function name** for global PHP functions (e.g. `my_hash_fn`) +- **`FullyQualifiedClass::method`** for static calls and instance method calls + (e.g. `Illuminate\Support\Facades\Hash::make`) + +Providing a non-empty list **completely replaces** the built-in allowlist, so +include any built-in entries you still want to keep: + +```neon +parameters: + sensitiveParameter: + cryptoCallees: + - password_hash + - password_verify + - hash + - hash_hmac + - 'Illuminate\Support\Facades\Hash::make' + - 'Illuminate\Support\Facades\Hash::check' + - 'App\Security\Hasher::hash' +``` ## Suppressing Warnings diff --git a/extension.neon b/extension.neon index 8a2526d..8483e25 100644 --- a/extension.neon +++ b/extension.neon @@ -1,14 +1,27 @@ +parametersSchema: + sensitiveParameter: structure([ + keywords: listOf(string()) + cryptoCallees: listOf(string()) + ]) + parameters: stubFiles: - stubs/SensitiveParameterValue.stub + sensitiveParameter: + keywords: [] + cryptoCallees: [] services: - class: LycheeOrg\PHPStan\Rules\SensitiveParameterDetectorRule + arguments: + sensitiveKeywords: %sensitiveParameter.keywords% tags: - phpstan.rules.rule - class: LycheeOrg\PHPStan\Rules\SensitiveParameterPropagationRule + arguments: + cryptoCallees: %sensitiveParameter.cryptoCallees% tags: - phpstan.rules.rule - diff --git a/src/Rules/SensitiveParameterPropagationRule.php b/src/Rules/SensitiveParameterPropagationRule.php index ee80958..e2e005e 100644 --- a/src/Rules/SensitiveParameterPropagationRule.php +++ b/src/Rules/SensitiveParameterPropagationRule.php @@ -44,8 +44,80 @@ */ final class SensitiveParameterPropagationRule implements Rule { - public function __construct(private ReflectionProvider $reflectionProvider) - { + /** + * Built-in cryptographic functions and methods that are designed to receive + * sensitive values. Passing a #[\SensitiveParameter] argument into these + * is the intended safe pattern and must never be flagged. + * + * Functions are listed by their fully-qualified name; methods by + * "FullyQualifiedClass::methodName". + */ + private const DEFAULT_CRYPTO_CALLEES = [ + // PHP built-in password functions + 'password_hash', + 'password_verify', + // PHP built-in hash functions + 'hash', + 'hash_hmac', + 'hash_pbkdf2', + 'hash_equals', + 'crypt', + 'md5', + 'sha1', + // OpenSSL + 'openssl_encrypt', + 'openssl_decrypt', + 'openssl_digest', + 'openssl_sign', + 'openssl_verify', + // Sodium + 'sodium_crypto_pwhash', + 'sodium_crypto_pwhash_str', + 'sodium_crypto_pwhash_str_verify', + 'sodium_crypto_secretbox', + 'sodium_crypto_secretbox_open', + 'sodium_crypto_auth', + 'sodium_crypto_auth_verify', + 'sodium_crypto_box', + 'sodium_crypto_box_open', + 'sodium_crypto_sign', + 'sodium_crypto_sign_open', + 'sodium_crypto_sign_detached', + 'sodium_crypto_sign_verify_detached', + 'sodium_crypto_aead_xchacha20poly1305_ietf_encrypt', + 'sodium_crypto_aead_xchacha20poly1305_ietf_decrypt', + 'sodium_crypto_generichash', + 'sodium_crypto_shorthash', + // Laravel Hash facade + 'Illuminate\Support\Facades\Hash::make', + 'Illuminate\Support\Facades\Hash::check', + 'Illuminate\Support\Facades\Hash::needsRehash', + // Laravel concrete hashing implementations + 'Illuminate\Hashing\BcryptHasher::make', + 'Illuminate\Hashing\BcryptHasher::check', + 'Illuminate\Hashing\ArgonHasher::make', + 'Illuminate\Hashing\ArgonHasher::check', + 'Illuminate\Hashing\Argon2IdHasher::make', + 'Illuminate\Hashing\Argon2IdHasher::check', + // LdapRecord authentication + 'LdapRecord\Auth\Guard::attempt', + ]; + + /** @var string[] */ + private array $cryptoCallees; + + /** + * @param string[] $cryptoCallees Fully-qualified function names or + * "ClassName::method" strings for callees that are designed to receive + * sensitive values (hashing, encryption, …) and must not trigger a + * propagation warning. When an empty array is provided the built-in + * defaults are used; supply a non-empty list to override them entirely. + */ + public function __construct( + private ReflectionProvider $reflectionProvider, + array $cryptoCallees = [], + ) { + $this->cryptoCallees = $cryptoCallees !== [] ? $cryptoCallees : self::DEFAULT_CRYPTO_CALLEES; } public function getNodeType(): string @@ -77,6 +149,10 @@ public function processNode(Node $node, Scope $scope): array $callerFunctionProtected = $this->hasSensitiveAttribute($callerFunction->getAttributes()); + if ($this->isSafeCallee($node, $scope)) { + return []; + } + $callee = $this->resolveCallee($node, $scope); if ($callee === null) { return []; @@ -132,6 +208,55 @@ public function processNode(Node $node, Scope $scope): array return $errors; } + /** + * Returns true when the callee is a known cryptographic / hashing function + * that is specifically designed to receive sensitive data, so no + * propagation error should be raised for it. + */ + private function isSafeCallee(CallLike $node, Scope $scope): bool + { + if ($node instanceof FuncCall && $node->name instanceof Node\Name) { + if (! $this->reflectionProvider->hasFunction($node->name, $scope)) { + return false; + } + + return in_array( + $this->reflectionProvider->getFunction($node->name, $scope)->getName(), + $this->cryptoCallees, + true, + ); + } + + $methodName = null; + $classNames = []; + + if ( + ($node instanceof MethodCall || $node instanceof NullsafeMethodCall) + && $node->name instanceof Node\Identifier + ) { + $methodName = $node->name->toString(); + $classNames = $scope->getType($node->var)->getObjectClassNames(); + } elseif ($node instanceof StaticCall && $node->name instanceof Node\Identifier) { + $methodName = $node->name->toString(); + $type = $node->class instanceof Node\Name + ? $scope->resolveTypeByName($node->class) + : $scope->getType($node->class); + $classNames = $type->getObjectClassNames(); + } + + if ($methodName === null) { + return false; + } + + foreach ($classNames as $className) { + if (in_array("{$className}::{$methodName}", $this->cryptoCallees, true)) { + return true; + } + } + + return false; + } + /** * @return array{0: AttributeReflection[], 1: ExtendedParameterReflection[]}|null */ diff --git a/tests/Fixtures/Propagation/CryptoPropagationCases.php b/tests/Fixtures/Propagation/CryptoPropagationCases.php new file mode 100644 index 0000000..b8025aa --- /dev/null +++ b/tests/Fixtures/Propagation/CryptoPropagationCases.php @@ -0,0 +1,45 @@ +analyse([__DIR__.'/../Fixtures/Propagation/CryptoPropagationCases.php'], []); +}); + +it('flags sensitive parameters passed to an unknown callee by default', function () { + $this->analyse([__DIR__.'/../Fixtures/Propagation/CustomHasherCases.php'], [ + [ + 'Parameter $password is marked #[\\SensitiveParameter] but is passed to a parameter ($value) that is not itself marked with #[\\SensitiveParameter]. Add the attribute there too or ignore with `@phpstan-ignore sensitiveParameter.propagation`.', + 21, + ], + ]); +}); + +it('does not flag sensitive parameters passed to a user-configured safe callee', function () { + $this->rule = new SensitiveParameterPropagationRule( + $this->createReflectionProvider(), + ['Tests\Fixtures\Propagation\CryptoHasher::hash'], + ); + + $this->analyse([__DIR__.'/../Fixtures/Propagation/CustomHasherCases.php'], []); +});