Add YAML security rules and tests for ldap3 and MariaDB #179
Conversation
WalkthroughThis pull request introduces several new YAML files that define security rules for Python applications using the Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer Code
participant Scanner as Security Scanner
Dev->>Scanner: Submit Python source code
Scanner->>Scanner: Parse AST for connection calls
alt ldap3.Connection detected
Scanner->>Scanner: Check for empty or hardcoded password
Scanner--)Dev: Emit warning if vulnerability found
else mariadb.connect detected
Scanner->>Scanner: Verify password is not empty/hardcoded
Scanner--)Dev: Emit security warning if issue found
end
Possibly related PRs
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
tests/python/python-ldap3-hardcoded-secret-python-test.yml (1)
10-10: Missing Newline at End of File
YAMLlint reports no newline at the end of the file. Please add a newline character at the end to satisfy formatting standards.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
tests/python/python-ldap3-empty-password-python-test.yml (1)
10-10: Missing Newline at End of File
A newline at the end of the file is missing as indicated by YAMLlint. Please add one to conform to the style guidelines.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
tests/__snapshots__/python-ldap3-empty-password-python-snapshot.yml (1)
30-78: Verification of Variable-Based Password Snapshot (Block 2)
This snapshot tests an instance where an empty string is assigned to a variable (test = "") and then passed as the password. The labels are comprehensive, though there is some repetitive capturing of the initialization (test = ""). If possible, consider streamlining the redundant label entries to improve clarity.tests/__snapshots__/python-mariadb-empty-password-python-snapshot.yml (1)
1-58: Review of MariaDB Connection Snapshot with Variable
This snapshot validates the case where an empty password is provided via a variable (PASSWORD1 = "") in the connection call. The labels offer exhaustive details (tracking the variable, the password keyword, and the empty string literal), although some entries appear repetitively. You might consider consolidating similar labels if they do not add extra matching value.tests/__snapshots__/python-mariadb-hardcoded-secret-python-snapshot.yml (2)
3-62: Review of Hardcoded Secret Snapshot with Variable
This snapshot validates the scenario where a hardcoded password is stored in a variable (PASSWORD1 = "test") and then used in a MariaDB connection. The labels are detailed and capture both the variable declaration and its usage in the connection call.
63-101: Review of Direct Hardcoded Password Snapshot
The snapshot that usesconn = mariadb.connect(password="test")clearly demonstrates the hardcoded secret case. Although the label definitions are exhaustive, there is minor redundancy in capturing the string literal portions.rules/python/security/python-ldap3-empty-password-python.yml (1)
16-60: Review of Utilities for Pattern Matching (Part 1)
The first utility block defines a pattern (ldap3.Connection(..., password="",...)_INSTANCE) to capture scenarios where an empty password is used. The nested conditions checking attributes and the argument list are comprehensive. Adding inline documentation or comments within this block might improve maintainability by clarifying the purpose of each nested condition.rules/python/security/python-ldap3-hardcoded-secret-python.yml (1)
32-55: Review of Password Definition Utility
Thedefine_passwordutility leverages both the previously defined string matching and an identifier pattern to detect password values. Although its nested structure is somewhat complex, consider adding inline comments to explain the matching logic for future maintainers.rules/python/security/python-mariadb-empty-password-python.yml (3)
18-31: Utility Pattern for Empty String Matching
Thedefine_stringutility is designed to detect an empty string by requiring a string start and end while ensuring no string content exists. This is a precise approach to identify an empty password.Consider adding inline comments to explain the purpose of each condition for future maintainers.
136-155: Attribute Access via Direct Module Call
The pattern here matches calls whereconnectis accessed as an attribute on themariadbmodule (e.g.mariadb.connect(...)). This provides additional coverage for detecting empty password usage in different coding styles.Consider whether this pattern could be consolidated with similar segments to reduce redundancy while maintaining clarity.
156-204: Detection with Aliased Import Matching Using $MARIADB_ALIAS
This segment is designed to detect cases whereconnectis called via an alias obtained from an aliased import, identified by$MARIADB_ALIAS. The use of theinsideclause (ensuring the call follows an import statement with an aliased import) adds robustness by confirming the source of the call.Refactoring common sub-patterns (possibly via YAML anchors or reusable blocks) might simplify future maintenance and reduce duplication across segments.
rules/python/security/python-mariadb-hardcoded-secret-python.yml (2)
136-155: Attribute Access Detection for Hardcoded Secrets
The rule here detects calls whereconnectis accessed via an attribute (e.g.mariadb.connect). This pattern ensures that even when the function is invoked in this style, any hard-coded password values are flagged.Similar to the empty password rule, consider consolidating overlapping detection patterns to avoid future maintenance challenges.
156-204: Aliased Import Matching Using $MARIADB_ALIAS for Hardcoded Secrets
This segment handles cases whereconnectis invoked through an alias provided by an aliased import (matched with$MARIADB_ALIAS). The use of an innerinsideclause to restrict the search to proper import statements increases the robustness of the detection.Investigate the possibility of reusing shared pattern definitions (for example, via YAML anchors) to reduce code duplication between the hardcoded secret and empty password rules.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
rules/python/security/python-ldap3-empty-password-python.yml(1 hunks)rules/python/security/python-ldap3-hardcoded-secret-python.yml(1 hunks)rules/python/security/python-mariadb-empty-password-python.yml(1 hunks)rules/python/security/python-mariadb-hardcoded-secret-python.yml(1 hunks)tests/__snapshots__/python-ldap3-empty-password-python-snapshot.yml(1 hunks)tests/__snapshots__/python-ldap3-hardcoded-secret-python-snapshot.yml(1 hunks)tests/__snapshots__/python-mariadb-empty-password-python-snapshot.yml(1 hunks)tests/__snapshots__/python-mariadb-hardcoded-secret-python-snapshot.yml(1 hunks)tests/python/python-ldap3-empty-password-python-test.yml(1 hunks)tests/python/python-ldap3-hardcoded-secret-python-test.yml(1 hunks)tests/python/python-mariadb-empty-password-python-test.yml(1 hunks)tests/python/python-mariadb-hardcoded-secret-python-test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
tests/python/python-ldap3-empty-password-python-test.yml
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
tests/python/python-ldap3-hardcoded-secret-python-test.yml
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (38)
tests/python/python-ldap3-hardcoded-secret-python-test.yml (2)
1-4: Valid Configuration and Readability
The valid example clearly uses a variable (test) for the password rather than a hardcoded string, which aligns with secure credential management practices.
5-10: Invalid Cases Clearly Specified
Both invalid examples—one with a hardcoded string ("test") and one with a variable assigned a hardcoded value—are clearly delineated. Consider adding inline documentation or comments referencing the relevant CWE/OWASP guidelines to aid future maintainers in understanding the security implications.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
tests/python/python-mariadb-empty-password-python-test.yml (4)
1-4: Valid Case for Empty Password Handling
The valid snippet uses an environment variable (os.env['pass']) to supply the password, which avoids hardcoding secrets. Verify that the class nameMySQLDatabeis intentional.
6-8: Invalid Example: Variable with Empty Password
Assigning an empty string to a variable (PASSWORD1) and passing it as the password correctly demonstrates the risk associated with empty credentials.
9-10: Invalid Example: Direct Empty Password
The direct use of an empty string in the connection function call effectively illustrates the issue.
11-13: Invalid Example: Alias with Empty Password
The snippet using an alias (mrdbl123) to connect with an empty password further reinforces that alternative import names do not circumvent the risk. Consider adding a brief comment to emphasize the security concern.tests/python/python-mariadb-hardcoded-secret-python-test.yml (4)
1-4: Valid Configuration Avoiding Hardcoded Secrets
The valid snippet correctly retrieves the password from an environment variable, thereby avoiding hardcoded credentials. Again, ensureMySQLDatabeis the intended class name for consistency.
6-8: Invalid Case: Hardcoded Password via Variable
Assigning a hardcoded password to a variable (PASSWORD1) and then using it in the connection demonstrates a clear security risk.
9-10: Invalid Case: Direct Hardcoded Password
Directly passing"test"to the connection function is correctly flagged as an insecure practice.
11-13: Invalid Case: Aliased Import with Hardcoded Credential
Even though the MariaDB module is imported with an alias, using a hardcoded credential (with the parameterpasswd) remains a security concern.tests/python/python-ldap3-empty-password-python-test.yml (3)
1-4: Valid Example for LDAP3 Connection
The valid snippet uses a variable (test) for the password, which is preferable to using an empty literal.
5-7: Invalid Example: Direct Empty Password
Using an empty string directly inldap3.Connection(line 7) illustrates an insecure configuration.
8-10: Invalid Example: Variable Assigned an Empty String
Assigning an empty string to a variable (test) and then using it in the connection call reinforces the concern.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 10-10: no new line character at the end of file
(new-line-at-end-of-file)
tests/__snapshots__/python-ldap3-hardcoded-secret-python-snapshot.yml (2)
1-42: Comprehensive Snapshot for Direct Hardcoded Secret
The first snapshot accurately captures the case of a hardcoded password (e.g.ldap3.Connection(password="test")) with detailed labels that specify token positions. This detailed mapping is very useful for precise AST matching and automated security checks.
43-101: Detailed Snapshot for Variable-Based Hardcoded Secret
The second snapshot, capturing the example where a variable is assigned a hardcoded value (test = "password") before being passed into the connection call, is well structured. The granular label breakdown aids in identifying potential security flaws. Be sure to update these positions if the rule logic or source formatting changes in the future.tests/__snapshots__/python-ldap3-empty-password-python-snapshot.yml (1)
1-29: Approval for LDAP3 Empty Password Snapshot (Block 1)
The snapshot clearly captures the use of an empty password directly in the call (ldap3.Connection(password="")). The detailed labels accurately delineate the various segments (e.g. the function call, the password keyword, and the empty string literal). Please verify that the specifiedstartandendindices precisely match the intended AST regions.tests/__snapshots__/python-mariadb-empty-password-python-snapshot.yml (2)
59-93: Review of Direct Empty Password Snapshot
The snapshot usingconn = mariadb.connect(password="")is clear and well-labeled. Ensure that the definedstartandendpositions correctly capture the relevant substrings from the source.
94-137: Review of Aliased Connection Snapshot
This snapshot tests the usage of an aliased MariaDB module (mariadb as mrdbl123). It comprehensively captures the connection call with additional parameters (host, user, and database), ensuring that alias-based invocations are also flagged.tests/__snapshots__/python-mariadb-hardcoded-secret-python-snapshot.yml (1)
102-149: Review of Aliased Hardcoded Secret Snapshot
This snapshot covers the scenario where the module is imported under an alias (mrdbl123) and the connection call uses a hardcoded password. The labels correctly reflect the structure of this call, ensuring that alias usage is taken into account by the security rule.rules/python/security/python-ldap3-empty-password-python.yml (3)
1-15: Review of Metadata and Description
The metadata section effectively details the risk of creating a database connection with an empty password. The message and references (including CWE-287 and the OWASP Secrets Management Cheat Sheet) provide clear context and rationale for the rule.
61-86: Review of Utilities for Pattern Matching (Part 2)
This block captures direct calls toldap3.Connectionwith an explicit empty string for the password. It neatly complements the previous utility definition to cover both variable-based and literal password cases.
87-100: Review of Final Security Rule Logic
The rule section combines the defined utility patterns using ananyoperator and appropriately excludes error conditions. This logical composition ensures that both patterns (instance-based and direct literal call) trigger the warning. Testing this rule against real-world code examples will help verify comprehensive coverage.rules/python/security/python-ldap3-hardcoded-secret-python.yml (3)
1-16: Review of Hardcoded Secret Rule Metadata
The metadata clearly outlines the dangers of embedding hard-coded secrets, with references to CWE-798 and OWASP A07:2021. The message is informative and emphasizes best practices for secret management.
18-30: Review of String Definition Utility
Thedefine_stringutility is well-crafted to match string literals by verifying the start, content, and end components. This clear separation supports accurate pattern matching in the subsequent rules.
56-154: Review of Final Hardcoded Secret Detection Rule
The rule combines multiple matching strategies to capture hard-coded secrets in connection calls, including both direct calls and those using aliased imports. The multi-pronged approach is robust and should effectively flag insecure patterns.rules/python/security/python-mariadb-empty-password-python.yml (6)
1-10: Metadata and Message Clarity Check
The metadata fields (id, severity, language) are clearly defined, and the multi-line message properly describes the risk of using an empty password when connecting to MariaDB. The message text is clear and aligns with CWE-287.
11-15: Note Field Accuracy and Reference
The note block correctly cites [CWE-287] and provides an external reference to OWASP's Secrets Management Cheat Sheet. This well supports the intended security advisory.
16-16: AST Essentials Flag Confirmation
Theast-grep-essentials: trueflag is set appropriately, ensuring that AST-based pattern matching is enabled.
32-55: Utility Pattern for Password Detection
Thedefine_passwordblock correctly combines two alternative matching strategies (either matching the empty string pattern viadefine_stringor matching an identifier pattern with$PWD_IDENTIFIERin a specific assignment context). The nested structure is clear and fulfills the detection requirements.
56-92: Direct Call Pattern Detection
This rule segment targets direct calls toconnect(identified by a regex match) where the keyword argument (namedpasswordorpasswd) is provided. The use of aninsideclause to restrict the match to an import-from statement from themariadbmodule helps in reducing false positives.
93-135: Aliased Call Pattern Matching Using $SASL_ALIAS
This segment captures cases where theconnectcall is relayed via an alias ($SASL_ALIAS). It mirrors the logic of the first segment while ensuring that the password parameter conforms to the defined password pattern.rules/python/security/python-mariadb-hardcoded-secret-python.yml (7)
1-10: Hardcoded Secret Metadata and Message Clarity
The metadata in this rule (id, language, severity) is clearly specified, and the multi-line message effectively communicates the risk of hard-coded secrets. It emphasizes the importance of using environment variables or secure vaults for managing sensitive data, aligning well with CWE-798 and OWASP guidelines.
11-15: Note Field for Hardcoded Credential Risks
The note field here includes critical references ([CWE-798] and [OWASP A07:2021]) along with an external reference. This correctly reinforces the secure coding practices intended by the rule.
16-16: AST Essentials Flag Confirmation
Theast-grep-essentials: trueflag is appropriately enabled for AST-based pattern matching.
18-30: Utility Pattern for Non-Empty String Matching
Thedefine_stringutility here is configured to match strings containing content (by verifying string_start, string_content, and string_end). This is suitable for detecting hard-coded secret values, distinguishing it from an empty string match.
32-55: Utility Pattern for Password Detection in Hardcoded Secrets
Thedefine_passwordblock leverages the non-empty string pattern to detect password literals. Its structure mirrors that of the empty password rule but is appropriately adapted to expect non-empty string content.
56-92: Direct Call Pattern for Hardcoded Secrets
This segment targets direct calls toconnectwhere the password keyword argument is passed. By usingdefine_password, it can detect hard-coded (non-empty) secret values. The match is further scoped by ensuring the call resides within an import-from statement from themariadbmodule.
93-135: Aliased Call Pattern Matching Using $SASL_ALIAS for Hardcoded Secrets
This segment precisely captures calls involving an alternative alias ($SASL_ALIAS), ensuring that passwords passed toconnectadhere to the hardcoded secret detection logic.
Summary by CodeRabbit
New Features
Tests