Skip to content

Java: Improve URI host allowlist checks - #22578

Merged
owen-mc merged 3 commits into
github:mainfrom
theinfosecguy:java-uri-host-allowlist
Oct 14, 2026
Merged

owen-mc merged 3 commits into
github:mainfrom
theinfosecguy:java-uri-host-allowlist

Conversation

@theinfosecguy

Copy link
Copy Markdown
Contributor

Recognize case-insensitive comparisons of URI.getHost() against constant string allowlist entries, including equality checks through local variables and casts. This reduces false positives in java/ssrf and java/unvalidated-url-redirection.

New matches require actual String comparison methods and preserve the connection between the checked host and the URI. Tests cover reassignment, different URIs, bypassed guards, and untrusted comparison values.

Partially addresses #22259. Stream and helper-based checks remain unresolved.

@owen-mc owen-mc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for this contribution. My apologies for the delay in reviewing it. It looks very close to being ready to merge.

The new tests that you've added should be in the existing test folders for these queries (java/ql/test/query-tests/security/CWE-601/semmle/tests/ and java/ql/test/query-tests/security/CWE-918/). You should keep them as new java files. The classpaths in the options files might need to be updated.

Also, one CI check is failing that would be fixed by merging in main, or rebasing on main, so please do that.

@theinfosecguy

Copy link
Copy Markdown
Contributor Author

Thank you for this contribution. My apologies for the delay in reviewing it. It looks very close to being ready to merge.

The new tests that you've added should be in the existing test folders for these queries (java/ql/test/query-tests/security/CWE-601/semmle/tests/ and java/ql/test/query-tests/security/CWE-918/). You should keep them as new java files. The classpaths in the options files might need to be updated.

Also, one CI check is failing that would be fixed by merging in main, or rebasing on main, so please do that.

No worries. Thanks for the review. I have made the requested changes. Can you please take a look?

@owen-mc owen-mc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for this improvement.

I plan to do a follow-up PR soon to integrate this better with the existing list of constants sanitizer, which applies to all queries, so that more queries benefit from some of this logic. Ideally we'd move most of this away from being query-specific.

@owen-mc
owen-mc merged commit dd4405c into github:main Oct 14, 2026
23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants