Skip to content

Cap nesting depth when extracting strings from user input - #719

Open
iacobdaniel wants to merge 4 commits into
mainfrom
fix/deeply-nested-user-input
Open

iacobdaniel wants to merge 4 commits into
mainfrom
fix/deeply-nested-user-input

Conversation

@iacobdaniel

Copy link
Copy Markdown
Contributor

A deeply nested value in a request made the string extraction recurse until it ran out of stack, and the scan skipped every detection for that request. The extraction walks at most 30 levels of nesting, and arrays nested deeper than that are no longer stringified. Real requests stay far below that depth, so nothing changes for them; anything deeper is inspected up to the limit and the rest of the request is still checked.

Comment thread aikido_zen/helpers/extract_strings_from_user_input.py Outdated
Comment on lines +47 to +48
if len(path_to_payload) >= MAX_TRAVERSAL_DEPTH:
return results, MAX_TRAVERSAL_DEPTH + 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High - Depth cap drops nested payloads before vulnerability checks

An attacker can place a malicious value at nesting depth 30 or greater in a request body or other structured user-input source that the application later uses in a SQL, shell, path, or SSRF sink. The new cutoff returns an empty result for that branch, and the production callers have no second traversal, so the corresponding detector never receives the payload and the request can reach the sink without being blocked or reported.

Show fix

Keep the stack-safe traversal bounded without silently dropping security-relevant leaves: use an iterative traversal or a bounded work queue that records values beyond the recursion limit, or reject/flag inputs exceeding the supported nesting depth before they reach application sinks. Ensure every detector receives an explicit result for truncated branches rather than treating omission as a clean scan.

More info - Reply on this comment to give feedback or ignore the issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no reason in a real app to go deeper than 30. There is no reason to check what goes deeper than that. That is the entire point of the fix to not slow down and process very deep requests.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 We were not able to ignore this issue because of the following reason:

You do not have the permission to ignore issues.

Co-authored-by: aikido-pr-checks[bot] <169896070+aikido-pr-checks[bot]@users.noreply.github.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants