From 3a8d042d0cef9564d3030df1ecdd2b1cdc819443 Mon Sep 17 00:00:00 2001 From: Coffee & Security <104401469+coffeeandsecurity@users.noreply.github.com> Date: Wed, 13 Nov 2024 02:05:29 +0530 Subject: [PATCH] Fixed issues related to -android flag --- dakshscra.py | 4 +- rules/scanning/platform/c/c.xml | 39 ++++++++-- rules/scanning/platform/common.xml | 15 ++-- rules/scanning/platform/dotnet/dotnet.xml | 18 +++-- rules/scanning/platform/java/java.xml | 15 ++-- .../{mobile/android => kotlin}/kotlin.xml | 0 .../platform/mobile/android/android.xml | 72 +++++++++++++++++++ rules/scanning/platform/php/php.xml | 11 +++ rules/scanning/platform/python/python.xml | 11 +++ rules/scanning/rulesconfig.xml | 5 +- 10 files changed, 162 insertions(+), 28 deletions(-) rename rules/scanning/platform/{mobile/android => kotlin}/kotlin.xml (100%) diff --git a/dakshscra.py b/dakshscra.py index b8eb389..896f2b6 100644 --- a/dakshscra.py +++ b/dakshscra.py @@ -69,8 +69,8 @@ # Check if results.file_types is set to "auto" if results.rule_file and results.rule_file.lower() == "auto": - # Set file_types to the output of getAvailableRules() - results.rule_file = rutils.getAvailableRules(exclude=["android", "common"]) + # Incase any rule is not stable that can be excluded from 'auto' mode using - exclude=["notsostable", "common"] + results.rule_file = rutils.getAvailableRules(exclude=["common"]) results.file_types = results.rule_file.lower() # Override filetypes argument # Remove duplicates in rule_file and file_types diff --git a/rules/scanning/platform/c/c.xml b/rules/scanning/platform/c/c.xml index c30e167..814d867 100644 --- a/rules/scanning/platform/c/c.xml +++ b/rules/scanning/platform/c/c.xml @@ -165,12 +165,39 @@ - Insecure Handling of Sensitive Information - \b(memset|mlock|setrlimit|unlink|remove)\s*\( - Detects functions used to handle or erase sensitive information improperly. - Failure to securely erase or restrict access to sensitive information (e.g., passwords, cryptographic keys) can lead to unintended leaks. - Store sensitive information in heap buffers, not stack buffers. Overwrite sensitive data before deletion using `memset()`. Use `mlock()` to prevent swapping and `setrlimit()` to disable core dumps. - Ensure sensitive data is managed securely, overwritten before deletion, and not stored in plaintext unnecessarily. + Improper Use of `memset` for Sensitive Data + \bmemset\s*\(\s*(password|key|secret|token|sensitive_data|.*_buffer).*?,.*?\) + Detects potential improper clearing of sensitive data. + Using `memset` for clearing sensitive data can be optimized away by compilers, leaving data in memory. + Ensure to use a secure method, such as `explicit_bzero` or similar, to clear sensitive data. Verify that sensitive variables (e.g., passwords, keys) are securely erased before freeing memory. + Check if the data cleared using `memset` is sensitive (passwords, keys, etc.) and confirm if a secure clearing method (like `explicit_bzero`) is implemented to prevent compiler optimizations. + + + + Improper Use of `mlock` for Sensitive Data + \bmlock\s*\(.*?\) + Detects improper use of `mlock` for securing sensitive data. + Failure to lock sensitive data in memory may expose it to swapping, potentially leaking to disk. + Use `mlock` on sensitive data buffers to prevent them from being swapped to disk. Ensure sufficient permissions and memory limits are set for the application. + Verify that sensitive data buffers are locked using `mlock` to prevent swapping. Confirm that the application handles cases where `mlock` fails gracefully. + + + + Improper Use of `setrlimit` for Core Dump Prevention + \bsetrlimit\s*\(\s*RLIMIT_CORE\s*,\s*0\s*\) + Detects improper handling of `setrlimit` for preventing core dumps. + Failure to disable core dumps might expose sensitive data during application crashes. + Use `setrlimit(RLIMIT_CORE, 0)` to disable core dumps for applications handling sensitive data. Test for successful application of limits. + Check if core dumps are disabled using `setrlimit(RLIMIT_CORE, 0)`. Ensure no sensitive data can leak through crash dumps in production environments. + + + + Unsafe File Deletion Using `unlink` or `remove` + \b(unlink|remove)\s*\(\s*(".*(password|key|secret).*?"|sensitive_file.*)\s*\) + Detects unsafe deletion of sensitive files. + Files containing sensitive data might still be recoverable after deletion without secure erasure. + Implement secure file deletion methods, such as overwriting the file contents before deletion. Avoid plain `unlink` or `remove` for sensitive files. + Ensure sensitive files are securely deleted using overwriting methods before unlinking or removing. Check if sensitive files are flagged correctly and handled securely. diff --git a/rules/scanning/platform/common.xml b/rules/scanning/platform/common.xml index 37a386c..fe33b3f 100644 --- a/rules/scanning/platform/common.xml +++ b/rules/scanning/platform/common.xml @@ -78,15 +78,17 @@ Reviewers should verify the secure handling of RSA private keys and assess if adequate measures are in place to safeguard their confidentiality and integrity. + - Information Disclosure: Stack Trace or Error Messages - (?i)Stacktrace - Detects potential information disclosure due to stack traces or error messages. - If this rule matches, it indicates the potential presence of stack traces or error messages in the code, which can provide valuable information to attackers and facilitate targeted attacks. - Developers should ensure that error handling mechanisms are properly implemented, and sensitive information is not exposed in error responses. - Reviewers should check for the presence of sensitive information in error responses and verify the implementation of proper error handling. + Insecure HTTP Communication + http:// + Detects the use of insecure HTTP communication. + Using HTTP exposes the application to man-in-the-middle attacks, allowing attackers to intercept or manipulate traffic. + Use HTTPS for all network communication to ensure secure data transmission. Avoid using HTTP unless explicitly required and justified by the use case. + Review all URLs or network requests across source code, configuration files, and external API references to ensure HTTPS is used instead of HTTP. + Sensitive Debug Information (?i)\.debug\s*=\s*True|\$debug\s*=\s*true|DEBUG\s*=\s*True|IsDebuggingEnabled\s*=\s*true @@ -96,6 +98,7 @@ Reviewers should check for debug-related configurations in application settings and ensure that they are disabled or set to false in production environments to protect sensitive information. + diff --git a/rules/scanning/platform/dotnet/dotnet.xml b/rules/scanning/platform/dotnet/dotnet.xml index 1be3dea..f5fb07d 100644 --- a/rules/scanning/platform/dotnet/dotnet.xml +++ b/rules/scanning/platform/dotnet/dotnet.xml @@ -83,6 +83,7 @@ Reviewers should check for the presence of untrusted data output involving HttpContext.Current.Request.QueryString. They should verify if developers have implemented proper input validation, output encoding, and secure data handling techniques to prevent security vulnerabilities. Reviewers should also assess the sensitivity of the output data and confirm the presence of appropriate security controls. + Path Traversal: Directory.GetFiles @@ -93,6 +94,7 @@ Reviewers should examine the usage of Directory.GetFiles and verify if proper input validation is performed to prevent path traversal vulnerabilities. They should assess the file path handling and confirm that user-supplied input is restricted to authorized directories, preventing unauthorized access to sensitive files. + Unsafe Method: Command Execution @@ -103,6 +105,7 @@ Reviewers should ensure that proper input validation and sanitization techniques are implemented for command execution methods. They should verify that developers have taken appropriate measures to prevent command injection vulnerabilities, such as using parameterized queries or prepared statements. Reviewers should also assess the sensitivity of the commands executed and confirm that relevant security controls are in place. + Unvalidated Redirects and Forwards @@ -154,14 +157,15 @@ - Information Disclosure: Stack Trace - (?i)Response\.Write\s*\(.*StackTrace - Detects the usage of Response.Write method that may inadvertently expose stack trace information. - If this rule matches, it indicates the potential vulnerability of using Response.Write method that may inadvertently expose stack trace information. Stack trace details can provide valuable information to attackers, aiding them in identifying potential vulnerabilities or weaknesses in the application. - Developers should avoid using Response.Write method to output stack trace information in a production environment. Stack traces should only be displayed for debugging purposes and should be properly handled and logged in an appropriate manner. Instead of using Response.Write, developers should consider using custom error pages or appropriate logging mechanisms to capture and handle error information securely. - Reviewers should ensure that Response.Write method is not used to output stack trace information in a production environment. They should verify that developers have implemented proper error handling and logging mechanisms to prevent the inadvertent exposure of sensitive information. Reviewers should also assess the sensitivity of the information exposed and confirm that relevant security controls are in place. + Stack Trace or Error Disclosure + (?i)(Response\.Write\s*\(.*StackTrace|ex\.ToString\(\)|Console\.WriteLine\(.*Exception.*\)|throw new Exception\(.+\)) + Detects unsafe error handling and stack trace exposure in .NET applications. + Using `Response.Write`, `ex.ToString()`, or similar methods to output exception details can expose sensitive stack trace information, aiding attackers in identifying vulnerabilities. + Developers should avoid exposing stack traces or detailed exceptions directly to users. Use secure logging frameworks (e.g., Serilog, NLog) and custom error pages in web applications. + Reviewers should ensure that stack traces and exceptions are not exposed to users and are logged securely. Verify proper error handling mechanisms in both web and non-web contexts. + API Route Prefix: Improper Usage @@ -172,6 +176,7 @@ Reviewers should check for the presence of [RoutePrefix] attribute in API controllers and assess the configuration for proper route prefixing. They should verify that developers have followed established routing conventions and consider any potential conflicts or security risks. Reviewers should also ensure that the routing configuration has been thoroughly tested to avoid functional errors. + Request Validation: Disabled Configuration @@ -190,6 +195,7 @@ Reviewers should check for the presence of insecure configuration of the ServerCertificateValidationCallback. They should assess if developers have followed best practices for certificate validation, such as verifying the certificate's chain of trust, checking for revocation status, and validating the server's hostname. Reviewers should also verify the usage of well-established libraries or frameworks that provide robust certificate validation mechanisms. + XSS Mitigation: HTML Encoding using Razor diff --git a/rules/scanning/platform/java/java.xml b/rules/scanning/platform/java/java.xml index 355f2e7..11d238e 100644 --- a/rules/scanning/platform/java/java.xml +++ b/rules/scanning/platform/java/java.xml @@ -170,6 +170,7 @@ Developers should ensure that the logger functions (error, warn, info, debug, trace) in Java code are used securely. They should carefully validate the logged data to prevent the inclusion of sensitive information. Additionally, developers should review and configure the logging framework to avoid the exposure of sensitive data through the logs. Reviewers should verify that the usage of logger functions (error, warn, info, debug, trace) in Java code is appropriate and adheres to secure coding practices. They should assess if proper validation mechanisms are in place to prevent the logging of sensitive information. Reviewers should also review the log configurations to ensure that log levels are set appropriately and sensitive data is not leaked through the logs. + Debugging and Logging Statements (System\.out\.print|System\.err\.print|System\.out\.println|System\.err\.println|log\.(debug|trace)) @@ -178,14 +179,16 @@ Developers should ensure that debugging and logging statements are not present in production code. They should review and remove or disable any logging statements that may log sensitive information. Additionally, developers should follow secure coding practices to prevent the accidental inclusion of debugging statements in production code. Reviewers should verify that debugging and logging statements are appropriately handled in production code. They should assess if sensitive information is inadvertently logged and if proper mechanisms are in place to prevent it. Reviewers should also evaluate the code review and release processes to ensure that debugging statements are removed or disabled before deploying to production environments. + - Stack Traces in Error Messages - (e\.printStackTrace|e\.getCause\.printStackTrace) - Detects the usage of printStackTrace() method in exception handling, which can expose sensitive information through error messages. - If this rule matches, it indicates the use of printStackTrace() method in exception handling, which can include detailed stack trace information in error messages. Stack traces may contain sensitive data or implementation details that could aid attackers in identifying vulnerabilities or understanding the system's internal structure. - Developers should avoid including stack traces in error messages displayed to users or logged in application logs. They should handle exceptions appropriately by providing meaningful error messages without revealing sensitive information. Developers should implement proper exception handling and logging practices. - Reviewers should verify that stack traces are not included in error messages presented to users or logged in application logs. They should assess if proper exception handling practices are followed, and error messages are appropriately sanitized to prevent the disclosure of sensitive information. Reviewers should also evaluate the logging mechanisms to ensure that stack traces are logged securely. + Stack Trace or Error Disclosure + (?i)(e\.printStackTrace\(\)|e\.getCause\(\)\.printStackTrace\(\)|System\.out\.print\(.*Exception.*\)|throw new \w+Exception\(.+\)) + Detects unsafe stack trace logging or error handling in Java code. + Directly printing stack traces or detailed exceptions (e.g., via `e.printStackTrace` or `e.getCause().printStackTrace`) can expose sensitive information, such as implementation details and file paths. + Developers should replace direct stack trace printing with centralized logging frameworks like Log4j or SLF4J, ensuring sensitive information is excluded from logs. + Reviewers should check for improper use of `printStackTrace` and `getCause().printStackTrace` and ensure exceptions are logged securely and not exposed in production environments. + Unencrypted Data Transmission over HTTP new\s*HttpGet|new\s*HttpPost|new\s*HttpPut|new\s*HttpDelete|new\s*HttpURLConnection diff --git a/rules/scanning/platform/mobile/android/kotlin.xml b/rules/scanning/platform/kotlin/kotlin.xml similarity index 100% rename from rules/scanning/platform/mobile/android/kotlin.xml rename to rules/scanning/platform/kotlin/kotlin.xml diff --git a/rules/scanning/platform/mobile/android/android.xml b/rules/scanning/platform/mobile/android/android.xml index 630fe3d..fafd77a 100644 --- a/rules/scanning/platform/mobile/android/android.xml +++ b/rules/scanning/platform/mobile/android/android.xml @@ -1,5 +1,6 @@ + Insecure Permissions android\.permission\.(WRITE_EXTERNAL_STORAGE|READ_EXTERNAL_STORAGE|INTERNET|ACCESS_FINE_LOCATION|ACCESS_COARSE_LOCATION) @@ -8,6 +9,7 @@ Developers should carefully review and minimize the permissions requested by the application. Only request permissions that are necessary for the app's functionality and follow the principle of least privilege. Reviewers should verify that the AndroidManifest.xml file requests only the permissions necessary for the application's functionality. They should ensure that unnecessary or overly broad permissions are not granted, as they can pose security risks. + Content Providers with No Permissions android:authorities\s*=|android:permission\s*=\s*"(?!android\.) @@ -16,6 +18,7 @@ Developers should specify appropriate permissions for content providers to restrict access to authorized applications or components. They should review the android:permission attribute and ensure that sensitive data is protected from unauthorized access. Reviewers should verify that content providers have appropriate permissions set to restrict access to authorized applications or components. They should ensure that sensitive data managed by content providers is protected from unauthorized access. + Exported Components android:exported\s*=\s*"(true|TRUE)" @@ -24,6 +27,7 @@ Developers should review the exported components in the AndroidManifest.xml file and ensure that only necessary components are exported. Components that do not need to be accessed by other applications should not be exported. Reviewers should verify that only necessary components are marked as exported in the AndroidManifest.xml file. They should ensure that sensitive functionality or data is not exposed to unauthorized entities through exported components. + Allow Backup android:allowBackup\s*=\s*"(true|TRUE)" @@ -32,6 +36,7 @@ Developers should carefully consider whether allowing backup of application data is necessary. If sensitive data is stored, it should be encrypted or excluded from backup using the android:allowBackup attribute set to "false". Reviewers should verify that sensitive data is not being backed up insecurely by checking the value of the allowBackup attribute in the AndroidManifest.xml file. They should ensure that sensitive data is properly protected and not exposed to unauthorized access through backup mechanisms. + Unprotected Components android:permission\s*=\s*"(?!android\.) @@ -40,6 +45,7 @@ Developers should specify appropriate permissions for components to ensure that only authorized entities can access them. Components that require specific permissions should have the android:permission attribute set accordingly. Reviewers should verify that components requiring specific permissions have the android:permission attribute set appropriately in the AndroidManifest.xml file. They should ensure that sensitive functionality is protected from unauthorized access. + Debuggable Application android:debuggable\s*=\s*"(true|TRUE)" @@ -48,6 +54,7 @@ Developers should ensure that the android:debuggable attribute is set to "false" in release builds to prevent exposing sensitive information or vulnerabilities. Debugging features should be disabled in production builds. Reviewers should verify that the android:debuggable attribute is set to "false" in release builds of the application to prevent exposing sensitive information or vulnerabilities. They should ensure that debugging features are disabled in production builds. + Unsafe Intent Filters android.intent.action.VIEW @@ -56,6 +63,7 @@ Developers should carefully configure intent filters to only allow expected data and actions. They should validate input and sanitize data to prevent unintended behavior or exploitation. Reviewers should verify that intent filters are properly configured to restrict access and prevent unintended behavior. They should ensure that input validation and data sanitization are implemented to mitigate potential security risks. + Unsafe Broadcast Receivers android.intent.action.BOOT_COMPLETED @@ -64,5 +72,69 @@ Developers should carefully review broadcast receivers and ensure that they handle sensitive actions securely. They should implement appropriate permission checks and validation to prevent unauthorized access or execution of malicious code. Reviewers should verify that broadcast receivers handling sensitive actions are securely implemented to prevent unauthorized access or execution of malicious code. They should ensure that appropriate permission checks and validation are in place to mitigate potential security risks. + + + Insecure Network Traffic + android:usesCleartextTraffic\s*=\s*"(true|TRUE)" + Detects the use of cleartext traffic in the application's manifest. + Allowing cleartext traffic (`usesCleartextTraffic="true"`) makes the application vulnerable to network eavesdropping and man-in-the-middle attacks. + Set android:usesCleartextTraffic to "false" to enforce secure communication using HTTPS. Configure a Network Security Config for exceptions. + Reviewers should verify the android:usesCleartextTraffic attribute in the AndroidManifest.xml and confirm that it is set to "false" or exceptions are appropriately defined in the Network Security Config. + + + + + + + Missing Obfuscation for Sensitive Classes + -keepclassmembers class .*password.*|.*secret.* + Detects missing obfuscation for sensitive classes or methods. + Lack of obfuscation for sensitive classes makes it easier for attackers to reverse engineer the application and extract sensitive information. + Ensure that sensitive classes and methods are obfuscated in the Proguard/R8 configuration to prevent reverse engineering. + Verify that the Proguard/R8 rules include obfuscation for sensitive classes and methods. + + + + + + + Debuggable Build Enabled + debuggable\s*=\s*true + Detects if a build configuration enables debugging for release builds. + Enabling debuggable builds for release may expose sensitive data and debugging information to attackers. + Ensure that debugging is disabled for production builds by setting debuggable=false in the release build configuration. + Check the Gradle build configurations to confirm that debuggable is not enabled for release builds. + + + + Outdated Dependencies + implementation\s+['"][\w\.-]+:\w+:\d+\.\d+\.\d+['"] + Detects dependency declarations with version numbers in the Gradle file. + Using outdated dependencies can introduce known vulnerabilities into the application. + Regularly update dependencies to the latest stable versions to mitigate security risks. Review changelogs and security advisories before updating. + Check the Gradle file for dependency declarations and verify that the specified versions are up-to-date. Cross-reference with known vulnerabilities databases (e.g., CVE databases). + + + + + + + Missing Content Descriptions + android:contentDescription\s*=\s*["']["'] + Detects UI elements with missing or empty content descriptions. + Missing content descriptions make the app inaccessible to users relying on assistive technologies like screen readers. + Provide meaningful content descriptions for all UI elements, especially images and buttons, to improve accessibility. + Check that all interactive or visual elements in layout XML files have appropriate content descriptions. + + + + Insecure WebView Configuration + WebView.*setJavaScriptEnabled\(true\) + Detects insecure WebView configurations that enable JavaScript. + Enabling JavaScript in WebView without additional security measures can expose the application to XSS attacks. + Restrict JavaScript usage in WebView and enable additional safeguards, such as strict URL validation. + Verify WebView configurations in layout XML files and associated Java/Kotlin files for secure settings. + diff --git a/rules/scanning/platform/php/php.xml b/rules/scanning/platform/php/php.xml index e502b06..03e0743 100644 --- a/rules/scanning/platform/php/php.xml +++ b/rules/scanning/platform/php/php.xml @@ -111,6 +111,17 @@ + + + Stack Trace or Error Disclosure + (?i)(echo .*Exception.*;|print_r\(.*Exception.*\)|trigger_error\(.+\)) + Detects unsafe practices in error handling or logging in PHP applications. + Printing exceptions or using `trigger_error` with sensitive details can expose implementation specifics and file paths to attackers. + Developers should use secure error handling mechanisms, such as custom error handlers or structured logging, without exposing stack traces. + Reviewers should ensure sensitive error information is not printed or exposed and that proper error handling mechanisms are implemented. + + + Deprecated and Potentially Insecure SQL Functions diff --git a/rules/scanning/platform/python/python.xml b/rules/scanning/platform/python/python.xml index 3765e55..2135bb7 100644 --- a/rules/scanning/platform/python/python.xml +++ b/rules/scanning/platform/python/python.xml @@ -86,6 +86,17 @@ + + + Stack Trace or Error Disclosure + (?i)(traceback\.format_exc\(\)|traceback\.print_exc\(\)|print\(.*Exception.*\)) + Detects code that exposes stack traces or sensitive error information in Python applications. + Using `traceback.print_exc()` or printing exceptions directly can expose sensitive details like file paths, method names, and line numbers to attackers. + Developers should use structured logging frameworks like Python's `logging` module to capture exceptions securely without exposing them to end-users. + Reviewers should ensure that sensitive stack trace information is not printed or exposed to end-users and verify proper use of logging practices. + + + Insecure SQL Queries: raw and extra diff --git a/rules/scanning/rulesconfig.xml b/rules/scanning/rulesconfig.xml index 9bb7beb..c93acfd 100644 --- a/rules/scanning/rulesconfig.xml +++ b/rules/scanning/rulesconfig.xml @@ -29,7 +29,7 @@ kotlin - /mobile/android/kotlin.xml + /kotlin/kotlin.xml *.kt build.gradle, settings.gradle, gradle.properties *.java, *.xml, *.json, *.gradle, *.properties, *.log @@ -65,7 +65,8 @@ android /mobile/android/android.xml - *.java, *.kt, *.groovy + AndroidManifest.xml, build.gradle, proguard-rules.pro + AndroidManifest.xml, build.gradle, proguard-rules.pro *.xml, *.json, *.gradle, *.properties, *.log