TCE-1285 Support IDOR vulnerabilities

[DMarinhoCodacy]

No description provided.

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

While the PR is technically 'up to standards' according to automated metrics, it contains significant implementation gaps and logic errors in the rule definitions that should prevent merging in its current state. Specifically, the patterns for PHP and Spring contain syntax errors (literal string usage) that will prevent them from matching real-world code.

Furthermore, there is a total absence of test fixtures or sample code to verify these rules. Multiple acceptance criteria related to authorization detection and parameter handling are either incorrectly implemented or too broad, which will lead to high false-positive rates in generic patterns and missed detections in Flask and Spring due to unsupported URI template variations.

About this PR

• There is a recurring issue across several language rules where metavariables are treated as literal strings (e.g., in Java and PHP). This indicates a systemic misunderstanding of the pattern syntax that needs to be addressed across the entire ruleset.
• The PR description is empty, and no test files or fixtures were included to verify the logic of these new patterns. Given the complexity of AST-based rules, including test cases for both expected hits and expected misses (to avoid false positives) is essential.

Test suggestions

• Missing recommended test scenario: Verify Express rule triggers when 'req.params.id' is passed directly to 'findById'
• Missing recommended test scenario: Verify Spring rule triggers when a @PathVariable is used as a parameter in a Repository 'findById' call
• Missing recommended test scenario: Verify Flask rule triggers when a route argument is used in 'session.query().filter_by()'
• Missing recommended test scenario: Verify PHP rule triggers when $_GET data is concatenated into a query string

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Missing recommended test scenario: Verify Express rule triggers when 'req.params.id' is passed directly to 'findById'
2. Missing recommended test scenario: Verify Spring rule triggers when a @PathVariable is used as a parameter in a Repository 'findById' call
3. Missing recommended test scenario: Verify Flask rule triggers when a route argument is used in 'session.query().filter_by()'
4. Missing recommended test scenario: Verify PHP rule triggers when $_GET data is concatenated into a query string

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🟡 MEDIUM RISK}

The pattern uses a literal string "$PARAM", which will only match code that literally contains that text. To match any parameter name, use the metavariable without quotes:

Suggested change

	$QUERY.setParameter("$PARAM", $ID)
	$QUERY.setParameter($PARAM, $ID)

[codacy-production]

_{🟡 MEDIUM RISK}

The PHP patterns contain several logic errors and metadata gaps. Array keys (e.g., $_GET['$ID']) are using literal string quotes instead of metavariables, and the select pattern is overly restrictive regarding assignments. Additionally, while these detect IDOR risks, the use of string concatenation with user input is primarily a SQL Injection (CWE-89) vulnerability; consider adding CWE-89 to the metadata.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: The Flask route pattern '$ROUTE/<$ID>' only matches parameters without type converters. Flask routes frequently use converters like 'int:id' or 'uuid:id'. Consider adding patterns to cover these typed parameters to ensure user-controlled endpoints are correctly monitored.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: The Spring mapping pattern '$PATH/{$ID}' does not account for URI templates with regex constraints (e.g., '{id:[0-9]+}'). Adding patterns that support these variations will improve the rule's effectiveness in real-world Spring applications.

[codacy-production]

_{🟡 MEDIUM RISK}

Suggestion: This generic pattern is overly broad and likely to produce significant false positives. It flags any function that retrieves a resource by ID and returns it, regardless of whether authorization checks are performed earlier in the function body or if it is a legitimate service layer call. Consider refining the rule to focus only on identifiers explicitly sourced from request objects to reduce noise.