diff --git a/lib/src/analyzers/code/rules/auth/avoid_empty_catch.dart b/lib/src/analyzers/code/rules/auth/avoid_empty_catch.dart new file mode 100644 index 0000000..3a1df04 --- /dev/null +++ b/lib/src/analyzers/code/rules/auth/avoid_empty_catch.dart @@ -0,0 +1,85 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects empty catch blocks that silently swallow exceptions. +/// +/// Empty catch blocks can hide security issues and make debugging difficult. +class AvoidEmptyCatch extends AnalysisRule { + AvoidEmptyCatch() + : super( + name: 'avoid_empty_catch', + description: + 'Avoid empty catch blocks that silently swallow exceptions.', + ); + + static const LintCode code = LintCode( + 'avoid_empty_catch', + 'Empty catch block silently swallows exceptions.', + correctionMessage: + 'Handle the exception, log it, or rethrow it. ' + 'Never silently ignore exceptions.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addCatchClause(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + @override + void visitCatchClause(CatchClause node) { + final body = node.body; + final statements = body.statements; + + // Check if the catch block is empty or contains only comments + if (statements.isEmpty) { + rule.reportAtNode(node); + return; + } + + // Check if all statements are empty statements or just have no effect + final hasEffectiveStatement = statements.any((stmt) { + // Empty statement + if (stmt is EmptyStatement) return false; + + // Expression statement with just a variable reference (no effect) + if (stmt is ExpressionStatement) { + final expr = stmt.expression; + // Check for rethrow, throw, return, method calls, etc. + if (expr is RethrowExpression) return true; + if (expr is ThrowExpression) return true; + if (expr is MethodInvocation) return true; + if (expr is FunctionExpressionInvocation) return true; + if (expr is AssignmentExpression) return true; + // Simple identifier alone has no effect + if (expr is SimpleIdentifier) return false; + } + + if (stmt is ReturnStatement) return true; + if (stmt is IfStatement) return true; + if (stmt is Block) return true; + if (stmt is VariableDeclarationStatement) return true; + + return false; + }); + + if (!hasEffectiveStatement) { + rule.reportAtNode(node); + } + } +} diff --git a/lib/src/analyzers/code/rules/injection/avoid_dynamic_sql_queries.dart b/lib/src/analyzers/code/rules/injection/avoid_dynamic_sql_queries.dart new file mode 100644 index 0000000..0a0b19d --- /dev/null +++ b/lib/src/analyzers/code/rules/injection/avoid_dynamic_sql_queries.dart @@ -0,0 +1,82 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects dynamic SQL queries with string interpolation. +/// +/// CWE-89: SQL Injection +class AvoidDynamicSqlQueries extends AnalysisRule { + AvoidDynamicSqlQueries() + : super( + name: 'avoid_dynamic_sql_queries', + description: + 'Avoid dynamic SQL queries with string interpolation to ' + 'prevent SQL injection.', + ); + + static const LintCode code = LintCode( + 'avoid_dynamic_sql_queries', + 'Avoid dynamic SQL queries with string interpolation.', + correctionMessage: + 'Use parameterized queries or prepared statements ' + 'instead of string interpolation.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addStringInterpolation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + static const _sqlKeywords = [ + 'SELECT', + 'INSERT', + 'UPDATE', + 'DELETE', + 'DROP', + 'CREATE', + 'ALTER', + 'TRUNCATE', + ]; + + @override + void visitStringInterpolation(StringInterpolation node) { + // Get the full string content + final buffer = StringBuffer(); + for (final element in node.elements) { + if (element is InterpolationString) { + buffer.write(element.value); + } else if (element is InterpolationExpression) { + buffer.write('\${}'); // Placeholder for interpolation + } + } + final stringValue = buffer.toString().toUpperCase(); + + // Check if the string looks like a SQL query + final hasSqlKeyword = _sqlKeywords.any( + (keyword) => stringValue.contains(keyword), + ); + + // Check if it has interpolation (not just a static SQL string) + final hasInterpolation = + node.elements.any((e) => e is InterpolationExpression); + + if (hasSqlKeyword && hasInterpolation) { + rule.reportAtNode(node); + } + } +} diff --git a/lib/src/analyzers/code/rules/logging/avoid_logging_sensitive_data.dart b/lib/src/analyzers/code/rules/logging/avoid_logging_sensitive_data.dart new file mode 100644 index 0000000..41d6644 --- /dev/null +++ b/lib/src/analyzers/code/rules/logging/avoid_logging_sensitive_data.dart @@ -0,0 +1,92 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects logging/printing of sensitive data. +/// +/// CWE-532: Insertion of Sensitive Information into Log File +class AvoidLoggingSensitiveData extends AnalysisRule { + AvoidLoggingSensitiveData() + : super( + name: 'avoid_logging_sensitive_data', + description: 'Avoid logging sensitive data like passwords or tokens.', + ); + + static const LintCode code = LintCode( + 'avoid_logging_sensitive_data', + 'Avoid logging sensitive data like passwords or tokens.', + correctionMessage: + 'Remove sensitive data from log statements or use ' + 'redaction before logging.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + static const _sensitivePatterns = [ + 'password', + 'passwd', + 'token', + 'secret', + 'apikey', + 'api_key', + 'credential', + 'privatekey', + 'private_key', + 'accesstoken', + 'access_token', + 'authtoken', + 'auth_token', + 'auth', + ]; + + static const _loggingFunctions = [ + 'print', + 'debugPrint', + 'log', + 'logger', + 'info', + 'debug', + 'warning', + 'error', + 'severe', + ]; + + @override + void visitMethodInvocation(MethodInvocation node) { + final methodName = node.methodName.name.toLowerCase(); + + // Check if this is a logging function + if (!_loggingFunctions.any((f) => methodName.contains(f))) { + return; + } + + // Check arguments for sensitive variable names + for (final arg in node.argumentList.arguments) { + if (arg is SimpleIdentifier) { + final argName = arg.name.toLowerCase(); + if (_sensitivePatterns.any((p) => argName.contains(p))) { + rule.reportAtNode(node); + return; + } + } + } + } +} diff --git a/lib/src/analyzers/code/rules/network/avoid_certificate_pinning_bypass.dart b/lib/src/analyzers/code/rules/network/avoid_certificate_pinning_bypass.dart new file mode 100644 index 0000000..a514f0f --- /dev/null +++ b/lib/src/analyzers/code/rules/network/avoid_certificate_pinning_bypass.dart @@ -0,0 +1,78 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects bypassing of certificate pinning/validation. +/// +/// CWE-295: Improper Certificate Validation +class AvoidCertificatePinningBypass extends AnalysisRule { + AvoidCertificatePinningBypass() + : super( + name: 'avoid_certificate_pinning_bypass', + description: 'Avoid bypassing SSL certificate validation.', + ); + + static const LintCode code = LintCode( + 'avoid_certificate_pinning_bypass', + 'Avoid bypassing SSL certificate validation.', + correctionMessage: + 'Implement proper certificate validation instead of ' + 'returning true unconditionally.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addAssignmentExpression(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + @override + void visitAssignmentExpression(AssignmentExpression node) { + // Check for badCertificateCallback assignment + final leftSide = node.leftHandSide; + String? propertyName; + + if (leftSide is PropertyAccess) { + propertyName = leftSide.propertyName.name; + } else if (leftSide is PrefixedIdentifier) { + propertyName = leftSide.identifier.name; + } + + if (propertyName != 'badCertificateCallback') { + return; + } + + // Check if the right side is a function that returns true + final rightSide = node.rightHandSide; + if (_returnsTrueUnconditionally(rightSide)) { + rule.reportAtNode(node); + } + } + + bool _returnsTrueUnconditionally(Expression expr) { + if (expr is FunctionExpression) { + final body = expr.body; + if (body is ExpressionFunctionBody) { + final expression = body.expression; + if (expression is BooleanLiteral && expression.value) { + return true; + } + } + } + return false; + } +} diff --git a/lib/src/analyzers/code/rules/rules.dart b/lib/src/analyzers/code/rules/rules.dart index 5a51d43..eac0539 100644 --- a/lib/src/analyzers/code/rules/rules.dart +++ b/lib/src/analyzers/code/rules/rules.dart @@ -1,14 +1,33 @@ import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:dart_shield/src/analyzers/code/rules/auth/avoid_empty_catch.dart'; import 'package:dart_shield/src/analyzers/code/rules/cryptography/avoid_weak_hashing.dart'; import 'package:dart_shield/src/analyzers/code/rules/cryptography/prefer_secure_random.dart'; +import 'package:dart_shield/src/analyzers/code/rules/injection/avoid_dynamic_sql_queries.dart'; +import 'package:dart_shield/src/analyzers/code/rules/logging/avoid_logging_sensitive_data.dart'; +import 'package:dart_shield/src/analyzers/code/rules/network/avoid_certificate_pinning_bypass.dart'; import 'package:dart_shield/src/analyzers/code/rules/network/avoid_harcoded_urls.dart'; import 'package:dart_shield/src/analyzers/code/rules/network/prefer_https_over_http.dart'; import 'package:dart_shield/src/analyzers/code/rules/secrets/avoid_hardcoded_secrets.dart'; +import 'package:dart_shield/src/analyzers/code/rules/storage/avoid_insecure_file_storage.dart'; +import 'package:dart_shield/src/analyzers/code/rules/storage/avoid_shared_preferences_for_secrets.dart'; final List rules = [ - AvoidHardcodedSecrets(), - AvoidHardcodedUrls(), + // Auth + AvoidEmptyCatch(), + // Cryptography AvoidWeakHashing(), - PreferHttpsOverHttp(), PreferSecureRandom(), + // Injection + AvoidDynamicSqlQueries(), + // Logging + AvoidLoggingSensitiveData(), + // Network + AvoidCertificatePinningBypass(), + AvoidHardcodedUrls(), + PreferHttpsOverHttp(), + // Secrets + AvoidHardcodedSecrets(), + // Storage + AvoidInsecureFileStorage(), + AvoidSharedPreferencesForSecrets(), ]; diff --git a/lib/src/analyzers/code/rules/storage/avoid_insecure_file_storage.dart b/lib/src/analyzers/code/rules/storage/avoid_insecure_file_storage.dart new file mode 100644 index 0000000..043b005 --- /dev/null +++ b/lib/src/analyzers/code/rules/storage/avoid_insecure_file_storage.dart @@ -0,0 +1,74 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects writing sensitive data to files with sensitive names. +/// +/// CWE-922: Insecure Storage of Sensitive Information +class AvoidInsecureFileStorage extends AnalysisRule { + AvoidInsecureFileStorage() + : super( + name: 'avoid_insecure_file_storage', + description: + 'Avoid storing sensitive data in files with names that ' + 'suggest sensitive content.', + ); + + static const LintCode code = LintCode( + 'avoid_insecure_file_storage', + 'Avoid writing sensitive data to files without encryption.', + correctionMessage: + 'Encrypt sensitive data before writing to files, or use ' + 'a secure storage solution.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addInstanceCreationExpression(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + static const _sensitiveFilePatterns = [ + 'password', + 'passwd', + 'secret', + 'credential', + 'token', + 'key', + 'private', + 'auth', + ]; + + @override + void visitInstanceCreationExpression(InstanceCreationExpression node) { + // Check if this is a File constructor + final constructorName = node.constructorName.type.element?.name; + if (constructorName != 'File') return; + + // Check the file path argument for sensitive patterns + final args = node.argumentList.arguments; + if (args.isEmpty) return; + + final firstArg = args.first; + if (firstArg is SimpleStringLiteral) { + final filePath = firstArg.value.toLowerCase(); + if (_sensitiveFilePatterns.any((p) => filePath.contains(p))) { + rule.reportAtNode(node); + } + } + } +} diff --git a/lib/src/analyzers/code/rules/storage/avoid_shared_preferences_for_secrets.dart b/lib/src/analyzers/code/rules/storage/avoid_shared_preferences_for_secrets.dart new file mode 100644 index 0000000..f7eec25 --- /dev/null +++ b/lib/src/analyzers/code/rules/storage/avoid_shared_preferences_for_secrets.dart @@ -0,0 +1,90 @@ +import 'package:analyzer/analysis_rule/analysis_rule.dart'; +import 'package:analyzer/analysis_rule/rule_context.dart'; +import 'package:analyzer/analysis_rule/rule_visitor_registry.dart'; +import 'package:analyzer/dart/ast/ast.dart'; +import 'package:analyzer/dart/ast/visitor.dart'; +import 'package:analyzer/error/error.dart'; + +/// Detects storing sensitive data in SharedPreferences. +/// +/// CWE-312: Cleartext Storage of Sensitive Information +class AvoidSharedPreferencesForSecrets extends AnalysisRule { + AvoidSharedPreferencesForSecrets() + : super( + name: 'avoid_shared_preferences_for_secrets', + description: + 'Avoid storing sensitive data in SharedPreferences as it ' + 'is not encrypted.', + ); + + static const LintCode code = LintCode( + 'avoid_shared_preferences_for_secrets', + 'Avoid storing sensitive data in SharedPreferences (unencrypted).', + correctionMessage: + 'Use flutter_secure_storage or similar encrypted ' + 'storage for sensitive data.', + ); + + @override + DiagnosticCode get diagnosticCode => code; + + @override + void registerNodeProcessors( + RuleVisitorRegistry registry, + RuleContext context, + ) { + final visitor = _Visitor(rule: this); + registry.addMethodInvocation(this, visitor); + } +} + +class _Visitor extends SimpleAstVisitor { + _Visitor({required this.rule}); + final AnalysisRule rule; + + static const _sensitiveKeyPatterns = [ + 'password', + 'passwd', + 'token', + 'secret', + 'apikey', + 'api_key', + 'credential', + 'privatekey', + 'private_key', + 'auth', + 'accesstoken', + 'access_token', + ]; + + @override + void visitMethodInvocation(MethodInvocation node) { + final methodName = node.methodName.name; + + // Only check setString, setInt, etc. (write operations) + if (!methodName.startsWith('set')) { + return; + } + + // Check if this is on SharedPreferences type + final target = node.target; + if (target == null) return; + + final targetType = target.staticType?.toString() ?? ''; + if (!targetType.contains('SharedPreferences')) { + return; + } + + // Check the first argument (key) for sensitive patterns + final args = node.argumentList.arguments; + if (args.isEmpty) return; + + final firstArg = args.first; + if (firstArg is SimpleStringLiteral) { + final keyValue = firstArg.value.toLowerCase(); + if (_sensitiveKeyPatterns.any((p) => keyValue.contains(p))) { + rule.reportAtNode(node); + } + } + } +}