github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.52k stars 1.5k forks source link

Java: queries about user-controlled URLs should be sanitized by enforcing a prefix #4530

Open pwntester opened 3 years ago

pwntester commented 3 years ago

This comes from a discussion with @smowton over here.

Certain categories such as Open Redirect, SSRF and Android WebView URL injection will benefit from a sanitizer which would clean the taint in case the dataflow goes through a string prefix operation (concatenation, format strings, string buffers, string writer ...). Since not controlling the begging of the URL will severely decrease the exploitability of these issues.

smowton commented 3 years ago

Added a descriptive title

smowton commented 3 years ago

Looks like as well as the XSS query's clause relating to user-controlled fetches, this should also apply to other URL-based queries like java/unvalidated-url-redirection. Go's version of the same query already filters in this way here: https://github.com/github/codeql-go/blob/main/ql/src/semmle/go/security/UrlConcatenation.qll#L94

github-actions[bot] commented 3 years ago

This issue is stale because it has been open 14 days with no activity. Comment or remove the stale label in order to avoid having this issue closed in 7 days.

neverlovelynn commented 3 years ago

I encountered the same problem when using codeql. Is this problem solved now?

smowton commented 3 years ago

Hah, I'd forgotten about this. I ported this from Go when promoting the Java SSRF query from experimental (https://github.com/github/codeql/pull/5587). That should be generalised to other queries concerned with a user-controlled URL.

ZH3FENG commented 3 years ago

Same as javascript, so many false-positive reports, particularly SSRF. Is there any filter in js?

pwntester commented 2 years ago

Another category that would benefit from this is JNDI injections where attackers need to control the beginning of the url /cc @atorralba