Skip to content

Commit 9959a3c

Browse files
committed
add legacy protocol handler support
1 parent b5e2582 commit 9959a3c

File tree

3 files changed

+104
-9
lines changed

3 files changed

+104
-9
lines changed

java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@ import java
1515
import semmle.code.java.dataflow.TaintTracking
1616
import semmle.code.java.dataflow.FlowSources
1717
import semmle.code.java.controlflow.Guards
18+
import semmle.code.java.security.ExternalProcess
1819

1920
/**
20-
* A call to Desktop.browse() method for handling
21-
* TODO(alan): also support legacy/generic cases like invoking "rundll32 url.dll,FileProtocolHandler"
21+
* A call to Desktop.browse() method. This is the platform-agnostic standard now.
22+
* NOTE(alan): the URLRedirect query will likely also trigger due to a little imprecision there
2223
*/
2324
class DesktopBrowseCall extends MethodCall {
2425
DesktopBrowseCall() {
@@ -27,8 +28,44 @@ class DesktopBrowseCall extends MethodCall {
2728
}
2829

2930
Expr getUrlArgument() { result = this.getArgument(0) }
31+
}
32+
33+
/**
34+
* Legacy Windows-specific command execution for protocol handling, based on https://imagej.net/ij/source/ij/plugin/BrowserLauncher.java and
35+
* seen in CVE-2022-43550. We match on the Windows shell command, since it more distinct and pervasive.
36+
*/
37+
class LegacyWindowsProtocolHandlerCall extends ArgumentToExec {
38+
LegacyWindowsProtocolHandlerCall() {
39+
// Single string: "rundll32 url.dll,FileProtocolHandler " + url
40+
this instanceof StringArgumentToExec and
41+
exists(StringLiteral sl |
42+
sl.getParent*() = this and
43+
sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*")
44+
)
45+
or
46+
// Array: {"rundll32", "url.dll,FileProtocolHandler", url}
47+
this.getType().(Array).getElementType() instanceof TypeString and
48+
exists(ArrayInit init, StringLiteral rundll, StringLiteral urldll |
49+
init = this.(ArrayCreationExpr).getInit() and
50+
rundll = init.getAnInit() and
51+
urldll = init.getAnInit() and
52+
rundll.getValue().regexpMatch("(?i)rundll32(\\.exe)?") and
53+
urldll.getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler")
54+
)
55+
}
3056

31-
string getDescription() { result = "Desktop.browse()" }
57+
Expr getUrlArgument() {
58+
// For single string exec, the URL is tainted into the concatenated string
59+
result = this and this instanceof StringArgumentToExec
60+
or
61+
// For arrays, find elements after "url.dll,FileProtocolHandler"
62+
exists(ArrayInit init, int urlIdx, int dllIdx |
63+
init = this.(ArrayCreationExpr).getInit() and
64+
result = init.getInit(urlIdx) and
65+
init.getInit(dllIdx).(StringLiteral).getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler") and
66+
urlIdx > dllIdx
67+
)
68+
}
3269
}
3370

3471
/**
@@ -66,10 +103,11 @@ module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig
66103

67104
predicate isSink(DataFlow::Node sink) {
68105
exists(DesktopBrowseCall call | sink.asExpr() = call.getUrlArgument())
106+
or
107+
exists(LegacyWindowsProtocolHandlerCall call | sink.asExpr() = call.getUrlArgument())
69108
}
70109

71110
predicate isBarrier(DataFlow::Node node) {
72-
// Consider sanitized if there's a guard checking the scheme
73111
node = DataFlow::BarrierGuard<isUrlSchemeCheck/3>::getABarrierNode()
74112
}
75113
}
@@ -81,11 +119,22 @@ import PotentiallyUnguardedProtocolHandlerFlow::PathGraph
81119

82120
from
83121
PotentiallyUnguardedProtocolHandlerFlow::PathNode source,
84-
PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, DesktopBrowseCall call
122+
PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, Expr call, string callType
85123
where
86124
PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and
87-
sink.getNode().asExpr() = call.getUrlArgument()
125+
(
126+
exists(DesktopBrowseCall dbc |
127+
call = dbc and
128+
sink.getNode().asExpr() = dbc.getUrlArgument() and
129+
callType = "Desktop.browse()"
130+
)
131+
or
132+
exists(LegacyWindowsProtocolHandlerCall whc |
133+
call = whc and
134+
sink.getNode().asExpr() = whc.getUrlArgument() and
135+
callType = "rundll32 url.dll,FileProtocolHandler"
136+
)
137+
)
88138
select call, source, sink,
89-
call.getDescription() +
90-
" is called with untrusted input from $@ without proper URL scheme validation.",
139+
callType + " is called with untrusted input from $@ without proper URL scheme validation.",
91140
source.getNode(), "this source"

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ edges
77
| PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 |
88
| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 |
99
| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 |
10+
| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 |
11+
| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 |
1012
nodes
1113
| PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
1214
| PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) |
@@ -19,8 +21,14 @@ nodes
1921
| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI |
2022
| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String |
2123
| PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri |
24+
| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
25+
| PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | semmle.label | ... + ... |
26+
| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | semmle.label | getParameter(...) : String |
27+
| PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | semmle.label | ... + ... |
2228
subpaths
2329
#select
2430
| 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 |
2531
| 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 |
2632
| 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 |
33+
| PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) | this source |
34+
| PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) | this source |

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,45 @@ public void safe3(String userInput) throws IOException, URISyntaxException {
5050
}
5151

5252
public void safe4() throws IOException, URISyntaxException {
53-
// hardcoded, no untrusted input
5453
Desktop.getDesktop().browse(new URI("https://example.com"));
5554
}
55+
56+
// rundll32 test cases
57+
public void bad4_rundll32(HttpServletRequest request) throws IOException {
58+
String url = request.getParameter("url");
59+
// Single string command with concatenation
60+
Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url);
61+
}
62+
63+
public void bad5_rundll32(String userInput) throws IOException {
64+
// Array-based command
65+
String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", userInput };
66+
Runtime.getRuntime().exec(cmd);
67+
}
68+
69+
public void bad6_rundll32(HttpServletRequest request) throws IOException {
70+
String url = request.getParameter("url");
71+
// ProcessBuilder with list
72+
ProcessBuilder pb = new ProcessBuilder("rundll32", "url.dll,FileProtocolHandler", url);
73+
pb.start();
74+
}
75+
76+
public void safe5_rundll32(HttpServletRequest request) throws IOException, URISyntaxException {
77+
String url = request.getParameter("url");
78+
URI uri = new URI(url);
79+
if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) {
80+
Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url);
81+
}
82+
}
83+
84+
public void safe6_rundll32(String userInput) throws IOException {
85+
if (userInput.startsWith("https://") || userInput.startsWith("http://")) {
86+
String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", userInput };
87+
Runtime.getRuntime().exec(cmd);
88+
}
89+
}
90+
91+
public void safe7_rundll32() throws IOException {
92+
Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler https://example.com");
93+
}
5694
}

0 commit comments

Comments
 (0)