tests/swift/aes-hardcoded-secret-swift-test.yml (2)

2-4: Consider adding a valid test case.

While it's correct that there should be no valid use cases for hardcoded AES secrets, including at least one valid test case can help ensure the rule doesn't produce false positives. Consider adding a test case that demonstrates the correct way to handle AES secrets, such as using environment variables or a secure key management system.

---

5-10: LGTM: Invalid test cases cover different scenarios.

The invalid test cases effectively demonstrate the use of hardcoded secrets in different ways, which is good for testing the rule. However, consider using a more realistic IV in the examples, as "123" is very short and might not represent real-world usage accurately.

You could update the IV to a more realistic value, for example:

-    try AES(key: password, iv: "123")
+    try AES(key: password, iv: "1234567890abcdef")
-    try AES(key: "hello", iv: "123")
+    try AES(key: "hello", iv: "1234567890abcdef")

tests/java/cookie-missing-samesite-java-test.yml (2)

3-7: LGTM: Valid test case demonstrates secure cookie configuration.

The test case correctly sets a cookie with both HttpOnly and SameSite=strict attributes, which are excellent security practices.

Consider adding the Secure flag to the cookie for additional security, especially if the application might be accessed over HTTPS:

-       response.setHeader("Set-Cookie", "key=value; HttpOnly; SameSite=strict");
+       response.setHeader("Set-Cookie", "key=value; HttpOnly; Secure; SameSite=strict");

---

9-20: LGTM: Invalid test cases effectively demonstrate missing SameSite attribute.

Both test cases correctly show scenarios where the SameSite attribute is missing, which is the focus of this security rule. The use of different methods to set cookies (Cookie class and setHeader) provides good coverage.

To make the test cases more comprehensive, consider adding a third invalid case that sets a cookie with SameSite=None without the Secure flag, which is another insecure configuration:

- |
  @RequestMapping(value = "/cookie4", method = "GET")
  public void setInsecureSameSiteNoneCookie(@RequestParam String value, HttpServletResponse response) {
     response.setHeader("Set-Cookie", "key=value; SameSite=None");
  }

This addition would cover another important aspect of cookie security.

rules/java/security/cookie-missing-samesite-java.yml (5)

4-16: LGTM: Comprehensive and informative message.

The message provides a detailed explanation of the CSRF vulnerability and includes valuable prevention recommendations. The reference to the OWASP cheat sheet is particularly helpful.

Consider adding a brief mention of the specific risks associated with CSRF attacks (e.g., unauthorized actions performed on behalf of the user) to emphasize the importance of this check.

---

17-20: LGTM: Relevant references provided.

The inclusion of the CWE-352 reference and the Stack Overflow link about SameSite cookies in Java applications is helpful.

Consider adding links to official Java documentation or security best practices guides (e.g., OWASP Java Security Guide) to provide more authoritative resources alongside the community-driven Stack Overflow link.

---

21-63: LGTM: Well-structured rule patterns.

The rule patterns effectively target both setHeader and addCookie methods for cookie manipulation within the HttpServletResponse context. The AST-based pattern matching approach provides precise detection capabilities.

Consider adding a pattern to check for the use of third-party libraries or frameworks that might handle cookie creation, such as Spring Framework's ResponseCookie.Builder. This could help catch cases where developers are using higher-level abstractions for cookie management.

---

64-67: LGTM: Effective constraint for SameSite attribute.

The constraint correctly filters out cases where the SameSite attribute is already set or the cookie is null.

Consider expanding the regex to account for different case variations (e.g., 'samesite', 'SameSite', 'SAMESITE') to make the rule more robust against different implementation styles. For example:

regex: "(?i).*samesite=.*|null"

This case-insensitive regex will catch all variations of 'SameSite'.

---

1-67: Excellent work on this comprehensive CSRF prevention rule!

This rule effectively targets a critical security vulnerability by checking for missing SameSite attributes in Java web applications. The combination of clear messaging, relevant references, and precise AST-based pattern matching creates a powerful tool for identifying potential CSRF risks.

The rule's focus on both setHeader and addCookie methods, along with its context-aware constraints, should significantly reduce false positives while catching genuine vulnerabilities. The detailed explanation and prevention recommendations in the message will be particularly valuable for developers in understanding and addressing the issue.

To further enhance the effectiveness of this security rule across your codebase:

1. Consider integrating this rule into your CI/CD pipeline to catch potential CSRF vulnerabilities early in the development process.
2. Develop companion rules to check for proper implementation of CSRF tokens and other defense-in-depth mechanisms mentioned in the rule's message.
3. Create educational materials or workshops for your development team to ensure they understand the importance of CSRF prevention and how to leverage this rule effectively.

Great job on prioritizing security and creating such a well-crafted rule!

rules/swift/security/aes-hardcoded-secret-swift.yml (1)

1-285: Consider adding comments to utility patterns for improved documentation.

The rule is well-structured and comprehensive, effectively covering various scenarios of hardcoded AES key usage. To further enhance maintainability and ease of understanding, consider adding brief comments explaining the purpose and target scenario for each utility pattern.

For example, you could add comments like this:

# Matches direct AES usage within a try expression
match_pattern_try_expression_directly:
  # ... (existing code)

# Matches direct AES calls outside of try expressions
match_pattern_AES_statement_directly:
  # ... (existing code)

# Matches AES usage with an instance variable
match_pattern_AES_expression_with_instance:
  # ... (existing code)

Configuration used: CodeRabbit UI
Review profile: CHILL