Skip to content

Commit b5e2582

Browse files
ex0dus-0xex0dus-0x
committed
init: add PotentiallyUnguardedProtocolHandler query
1 parent a906346 commit b5e2582

File tree

5 files changed

+200
-0
lines changed

5 files changed

+200
-0
lines changed

‎java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<!DOCTYPE qhelp SYSTEM "qhelp.dtd">
2+
<qhelp>
3+
<overview>
4+
<p>
5+
URL protocol handlers are used to handle any URL scheme supported by the native desktop.
6+
If URL inputs to these handlers are untrusted and not properly sanitized, they can be
7+
used to perform unintended actions by another application registered to handle the same
8+
protocol.
9+
</p>
10+
</overview>
11+
<recommendation>
12+
<p>
13+
Review protocol handler sinks and ensure that any URL inputs are properly sanitized if
14+
they come from untrusted sources.
15+
</p>
16+
</recommendation>
17+
<references>
18+
<li> Positive Security: <a
19+
href="https://positive.security/blog/url-open-rce">Allow
20+
arbitrary URLs, expect arbitrary code execution</a>
21+
</li>
22+
<li> CVE-2022-43550: <a href="https://hackerone.com/reports/1692603">Jitsi Desktop Client
23+
RCE By Interacting with Malicious URL Schemes on Windows</a>
24+
</li>
25+
</references>
26+
</qhelp>

‎java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/**
2+
* @name Potentially unguarded protocol handler invocation
3+
* @id tob/java/unguarded-protocol-handler
4+
* @description Detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols
5+
* @kind path-problem
6+
* @tags security
7+
* external/cwe/cwe-939
8+
* @precision medium
9+
* @problem.severity warning
10+
* @security-severity 6.5
11+
* @group security
12+
*/
13+
14+
import java
15+
import semmle.code.java.dataflow.TaintTracking
16+
import semmle.code.java.dataflow.FlowSources
17+
import semmle.code.java.controlflow.Guards
18+
19+
/**
20+
* A call to Desktop.browse() method for handling
21+
* TODO(alan): also support legacy/generic cases like invoking "rundll32 url.dll,FileProtocolHandler"
22+
*/
23+
class DesktopBrowseCall extends MethodCall {
24+
DesktopBrowseCall() {
25+
this.getMethod().hasName("browse") and
26+
this.getMethod().getDeclaringType().hasQualifiedName("java.awt", "Desktop")
27+
}
28+
29+
Expr getUrlArgument() { result = this.getArgument(0) }
30+
31+
string getDescription() { result = "Desktop.browse()" }
32+
}
33+
34+
/**
35+
* A guard that checks the URL scheme/protocol signifying sanitization before launch
36+
*/
37+
predicate isUrlSchemeCheck(Guard g, Expr e, boolean branch) {
38+
exists(MethodCall mc |
39+
mc = g and
40+
e = mc.getQualifier*() and
41+
branch = true and
42+
(
43+
// getScheme() or getProtocol() check
44+
mc.getMethod().hasName(["equals", "equalsIgnoreCase", "startsWith", "matches"]) and
45+
exists(MethodCall getScheme |
46+
getScheme.getParent*() = mc and
47+
getScheme.getMethod().hasName(["getScheme", "getProtocol"])
48+
)
49+
or
50+
// URL string contains check like: url.contains("http://") or url.startsWith("https://")
51+
mc.getMethod().hasName(["contains", "startsWith", "matches", "indexOf"]) and
52+
exists(StringLiteral sl | sl = mc.getAnArgument() | sl.getValue().regexpMatch(".*://.*"))
53+
or
54+
// Pattern matching on the string representation
55+
mc.getMethod().hasName(["matches", "find"]) and
56+
mc.getQualifier().getType().(RefType).hasQualifiedName("java.util.regex", "Matcher")
57+
)
58+
)
59+
}
60+
61+
/**
62+
* Configuration for tracking untrusted data to protocol handler invocations
63+
*/
64+
module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig {
65+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
66+
67+
predicate isSink(DataFlow::Node sink) {
68+
exists(DesktopBrowseCall call | sink.asExpr() = call.getUrlArgument())
69+
}
70+
71+
predicate isBarrier(DataFlow::Node node) {
72+
// Consider sanitized if there's a guard checking the scheme
73+
node = DataFlow::BarrierGuard<isUrlSchemeCheck/3>::getABarrierNode()
74+
}
75+
}
76+
77+
module PotentiallyUnguardedProtocolHandlerFlow =
78+
TaintTracking::Global<PotentiallyUnguardedProtocolHandlerConfig>;
79+
80+
import PotentiallyUnguardedProtocolHandlerFlow::PathGraph
81+
82+
from
83+
PotentiallyUnguardedProtocolHandlerFlow::PathNode source,
84+
PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, DesktopBrowseCall call
85+
where
86+
PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and
87+
sink.getNode().asExpr() = call.getUrlArgument()
88+
select call, source, sink,
89+
call.getDescription() +
90+
" is called with untrusted input from $@ without proper URL scheme validation.",
91+
source.getNode(), "this source"

‎java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
edges
2+
| PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | provenance | Src:MaD:46190 |
3+
| PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | provenance | MaD:44428 Sink:MaD:43969 |
4+
| PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | provenance | Src:MaD:46190 |
5+
| PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | provenance | Sink:MaD:43969 |
6+
| PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | provenance | MaD:44428 |
7+
| PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 |
8+
| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 |
9+
| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 |
10+
nodes
11+
| PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
12+
| PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) |
13+
| PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | semmle.label | url : String |
14+
| PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
15+
| PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | semmle.label | new URI(...) : URI |
16+
| PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | semmle.label | url : String |
17+
| PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | semmle.label | uri |
18+
| PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
19+
| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI |
20+
| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String |
21+
| PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri |
22+
subpaths
23+
#select
24+
| PotentiallyUnguardedProtocolHandler.java:11:9:11:49 | browse(...) | PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | Desktop.browse() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) | this source |
25+
| PotentiallyUnguardedProtocolHandler.java:23:13:23:44 | browse(...) | PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | Desktop.browse() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) | this source |
26+
| PotentiallyUnguardedProtocolHandler.java:39:13:39:44 | browse(...) | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | Desktop.browse() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) | this source |

‎java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import java.awt.Desktop;
2+
import java.io.IOException;
3+
import java.net.URI;
4+
import java.net.URISyntaxException;
5+
import javax.servlet.http.HttpServletRequest;
6+
7+
public class PotentiallyUnguardedProtocolHandler {
8+
9+
public void bad1(HttpServletRequest request) throws IOException, URISyntaxException {
10+
String url = request.getParameter("url");
11+
Desktop.getDesktop().browse(new URI(url));
12+
}
13+
14+
public void bad2(String userInput) throws IOException, URISyntaxException {
15+
URI uri = new URI(userInput);
16+
Desktop.getDesktop().browse(uri);
17+
}
18+
19+
public void safe1(HttpServletRequest request) throws IOException, URISyntaxException {
20+
String url = request.getParameter("url");
21+
URI uri = new URI(url);
22+
if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) {
23+
Desktop.getDesktop().browse(uri);
24+
}
25+
}
26+
27+
public void safe2(String userInput) throws IOException, URISyntaxException {
28+
if (userInput.startsWith("https://") || userInput.startsWith("http://")) {
29+
Desktop.getDesktop().browse(new URI(userInput));
30+
}
31+
}
32+
33+
public void bad3(HttpServletRequest request) throws IOException, URISyntaxException {
34+
String url = request.getParameter("url");
35+
URI uri = new URI(url);
36+
37+
// Weak check - only checks if scheme exists, not what it is
38+
if (uri.getScheme() != null) {
39+
Desktop.getDesktop().browse(uri);
40+
}
41+
}
42+
43+
public void safe3(String userInput) throws IOException, URISyntaxException {
44+
URI uri = new URI(userInput);
45+
String scheme = uri.getScheme();
46+
47+
if (scheme != null && (scheme.equalsIgnoreCase("http") || scheme.equalsIgnoreCase("https"))) {
48+
Desktop.getDesktop().browse(uri);
49+
}
50+
}
51+
52+
public void safe4() throws IOException, URISyntaxException {
53+
// hardcoded, no untrusted input
54+
Desktop.getDesktop().browse(new URI("https://example.com"));
55+
}
56+
}

‎java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
security/PotentiallyUnguardedProtocolhandler/PotentiallyUnguardedProtocolhandler.ql

0 commit comments

Comments
 (0)