From 05212aaa62bb50ea5bcddb58ea5c2d11370103c4 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Wed, 17 Dec 2025 10:52:01 -0500 Subject: [PATCH 1/6] init: add PotentiallyUnguardedProtocolHandler query --- .../PotentiallyUnguardedProtocolHandler.qhelp | 26 ++++++ .../PotentiallyUnguardedProtocolHandler.ql | 91 +++++++++++++++++++ ...tentiallyUnguardedProtocolHandler.expected | 26 ++++++ .../PotentiallyUnguardedProtocolHandler.java | 56 ++++++++++++ .../PotentiallyUnguardedProtocolHandler.qlref | 1 + 5 files changed, 200 insertions(+) create mode 100644 java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp create mode 100644 java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql create mode 100644 java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected create mode 100644 java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java create mode 100644 java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref diff --git a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp new file mode 100644 index 0000000..1e63ba4 --- /dev/null +++ b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp @@ -0,0 +1,26 @@ + + + +

+ URL protocol handlers are used to handle any URL scheme supported by the native desktop. + If URL inputs to these handlers are untrusted and not properly sanitized, they can be + used to perform unintended actions by another application registered to handle the same + protocol. +

+ + +

+ Review protocol handler sinks and ensure that any URL inputs are properly sanitized if + they come from untrusted sources. +

+ + +
  • Positive Security: Allow + arbitrary URLs, expect arbitrary code execution +
    • +
  • CVE-2022-43550: Jitsi Desktop Client + RCE By Interacting with Malicious URL Schemes on Windows +
    • +     +     \ No newline at end of file diff --git a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql new file mode 100644 index 0000000..709a991 --- /dev/null +++ b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql @@ -0,0 +1,91 @@ +/** + * @name Potentially unguarded protocol handler invocation + * @id tob/java/unguarded-protocol-handler + * @description Detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols + * @kind path-problem + * @tags security + * external/cwe/cwe-939 + * @precision medium + * @problem.severity warning + * @security-severity 6.5 + * @group security + */ + +import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.controlflow.Guards + +/** + * A call to Desktop.browse() method for handling + * TODO(alan): also support legacy/generic cases like invoking "rundll32 url.dll,FileProtocolHandler" + */ +class DesktopBrowseCall extends MethodCall { + DesktopBrowseCall() { + this.getMethod().hasName("browse") and + this.getMethod().getDeclaringType().hasQualifiedName("java.awt", "Desktop") + } + + Expr getUrlArgument() { result = this.getArgument(0) } + + string getDescription() { result = "Desktop.browse()" } +} + +/** + * A guard that checks the URL scheme/protocol signifying sanitization before launch + */ +predicate isUrlSchemeCheck(Guard g, Expr e, boolean branch) { + exists(MethodCall mc | + mc = g and + e = mc.getQualifier*() and + branch = true and + ( + // getScheme() or getProtocol() check + mc.getMethod().hasName(["equals", "equalsIgnoreCase", "startsWith", "matches"]) and + exists(MethodCall getScheme | + getScheme.getParent*() = mc and + getScheme.getMethod().hasName(["getScheme", "getProtocol"]) + ) + or + // URL string contains check like: url.contains("http://") or url.startsWith("https://") + mc.getMethod().hasName(["contains", "startsWith", "matches", "indexOf"]) and + exists(StringLiteral sl | sl = mc.getAnArgument() | sl.getValue().regexpMatch(".*://.*")) + or + // Pattern matching on the string representation + mc.getMethod().hasName(["matches", "find"]) and + mc.getQualifier().getType().(RefType).hasQualifiedName("java.util.regex", "Matcher") + ) + ) +} + +/** + * Configuration for tracking untrusted data to protocol handler invocations + */ +module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { + exists(DesktopBrowseCall call | sink.asExpr() = call.getUrlArgument()) + } + + predicate isBarrier(DataFlow::Node node) { + // Consider sanitized if there's a guard checking the scheme + node = DataFlow::BarrierGuard::getABarrierNode() + } +} + +module PotentiallyUnguardedProtocolHandlerFlow = + TaintTracking::Global; + +import PotentiallyUnguardedProtocolHandlerFlow::PathGraph + +from + PotentiallyUnguardedProtocolHandlerFlow::PathNode source, + PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, DesktopBrowseCall call +where + PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and + sink.getNode().asExpr() = call.getUrlArgument() +select call, source, sink, + call.getDescription() + + " is called with untrusted input from $@ without proper URL scheme validation.", + source.getNode(), "this source" diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected new file mode 100644 index 0000000..8a66a01 --- /dev/null +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -0,0 +1,26 @@ +edges +| PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | provenance | Src:MaD:46190 | +| PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | provenance | MaD:44428 Sink:MaD:43969 | +| PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | provenance | Src:MaD:46190 | +| PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | provenance | Sink:MaD:43969 | +| PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | provenance | MaD:44428 | +| PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 | +| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 | +| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 | +nodes +| PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) | +| PotentiallyUnguardedProtocolHandler.java:11:45:11:47 | url : String | semmle.label | url : String | +| PotentiallyUnguardedProtocolHandler.java:20:22:20:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | semmle.label | new URI(...) : URI | +| PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | semmle.label | url : String | +| PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | semmle.label | uri | +| PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI | +| PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String | +| PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri | +subpaths +#select +| 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 | +| 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 | +| 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 | diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java new file mode 100644 index 0000000..9e8ac93 --- /dev/null +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java @@ -0,0 +1,56 @@ +import java.awt.Desktop; +import java.io.IOException; +import java.net.URI; +import java.net.URISyntaxException; +import javax.servlet.http.HttpServletRequest; + +public class PotentiallyUnguardedProtocolHandler { + + public void bad1(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + Desktop.getDesktop().browse(new URI(url)); + } + + public void bad2(String userInput) throws IOException, URISyntaxException { + URI uri = new URI(userInput); + Desktop.getDesktop().browse(uri); + } + + public void safe1(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + URI uri = new URI(url); + if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) { + Desktop.getDesktop().browse(uri); + } + } + + public void safe2(String userInput) throws IOException, URISyntaxException { + if (userInput.startsWith("https://") || userInput.startsWith("http://")) { + Desktop.getDesktop().browse(new URI(userInput)); + } + } + + public void bad3(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + URI uri = new URI(url); + + // Weak check - only checks if scheme exists, not what it is + if (uri.getScheme() != null) { + Desktop.getDesktop().browse(uri); + } + } + + public void safe3(String userInput) throws IOException, URISyntaxException { + URI uri = new URI(userInput); + String scheme = uri.getScheme(); + + if (scheme != null && (scheme.equalsIgnoreCase("http") || scheme.equalsIgnoreCase("https"))) { + Desktop.getDesktop().browse(uri); + } + } + + public void safe4() throws IOException, URISyntaxException { + // hardcoded, no untrusted input + Desktop.getDesktop().browse(new URI("https://example.com")); + } +} diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref new file mode 100644 index 0000000..bc3c1da --- /dev/null +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref @@ -0,0 +1 @@ +security/PotentiallyUnguardedProtocolhandler/PotentiallyUnguardedProtocolhandler.ql From 4664d38136eab92756be584113b393e2e0989b0c Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Wed, 17 Dec 2025 13:34:36 -0500 Subject: [PATCH 2/6] add legacy protocol handler support --- .../PotentiallyUnguardedProtocolHandler.ql | 65 ++++++++++++++++--- ...tentiallyUnguardedProtocolHandler.expected | 8 +++ .../PotentiallyUnguardedProtocolHandler.java | 40 +++++++++++- 3 files changed, 104 insertions(+), 9 deletions(-) diff --git a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql index 709a991..b234905 100644 --- a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql +++ b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql @@ -15,10 +15,11 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources import semmle.code.java.controlflow.Guards +import semmle.code.java.security.ExternalProcess /** - * A call to Desktop.browse() method for handling - * TODO(alan): also support legacy/generic cases like invoking "rundll32 url.dll,FileProtocolHandler" + * A call to Desktop.browse() method. This is the platform-agnostic standard now. + * NOTE(alan): the URLRedirect query will likely also trigger due to a little imprecision there */ class DesktopBrowseCall extends MethodCall { DesktopBrowseCall() { @@ -27,8 +28,44 @@ class DesktopBrowseCall extends MethodCall { } Expr getUrlArgument() { result = this.getArgument(0) } +} + +/** + * Legacy Windows-specific command execution for protocol handling, based on https://imagej.net/ij/source/ij/plugin/BrowserLauncher.java and + * seen in CVE-2022-43550. We match on the Windows shell command, since it more distinct and pervasive. + */ +class LegacyWindowsProtocolHandlerCall extends ArgumentToExec { + LegacyWindowsProtocolHandlerCall() { + // Single string: "rundll32 url.dll,FileProtocolHandler " + url + this instanceof StringArgumentToExec and + exists(StringLiteral sl | + sl.getParent*() = this and + sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") + ) + or + // Array: {"rundll32", "url.dll,FileProtocolHandler", url} + this.getType().(Array).getElementType() instanceof TypeString and + exists(ArrayInit init, StringLiteral rundll, StringLiteral urldll | + init = this.(ArrayCreationExpr).getInit() and + rundll = init.getAnInit() and + urldll = init.getAnInit() and + rundll.getValue().regexpMatch("(?i)rundll32(\\.exe)?") and + urldll.getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler") + ) + } - string getDescription() { result = "Desktop.browse()" } + Expr getUrlArgument() { + // For single string exec, the URL is tainted into the concatenated string + result = this and this instanceof StringArgumentToExec + or + // For arrays, find elements after "url.dll,FileProtocolHandler" + exists(ArrayInit init, int urlIdx, int dllIdx | + init = this.(ArrayCreationExpr).getInit() and + result = init.getInit(urlIdx) and + init.getInit(dllIdx).(StringLiteral).getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler") and + urlIdx > dllIdx + ) + } } /** @@ -66,10 +103,11 @@ module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig predicate isSink(DataFlow::Node sink) { exists(DesktopBrowseCall call | sink.asExpr() = call.getUrlArgument()) + or + exists(LegacyWindowsProtocolHandlerCall call | sink.asExpr() = call.getUrlArgument()) } predicate isBarrier(DataFlow::Node node) { - // Consider sanitized if there's a guard checking the scheme node = DataFlow::BarrierGuard::getABarrierNode() } } @@ -81,11 +119,22 @@ import PotentiallyUnguardedProtocolHandlerFlow::PathGraph from PotentiallyUnguardedProtocolHandlerFlow::PathNode source, - PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, DesktopBrowseCall call + PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, Expr call, string callType where PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and - sink.getNode().asExpr() = call.getUrlArgument() + ( + exists(DesktopBrowseCall dbc | + call = dbc and + sink.getNode().asExpr() = dbc.getUrlArgument() and + callType = "Desktop.browse()" + ) + or + exists(LegacyWindowsProtocolHandlerCall whc | + call = whc and + sink.getNode().asExpr() = whc.getUrlArgument() and + callType = "rundll32 url.dll,FileProtocolHandler" + ) + ) select call, source, sink, - call.getDescription() + - " is called with untrusted input from $@ without proper URL scheme validation.", + callType + " is called with untrusted input from $@ without proper URL scheme validation.", source.getNode(), "this source" diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected index 8a66a01..d8a7dac 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -7,6 +7,8 @@ edges | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 | | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 | +| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | nodes | PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) | @@ -19,8 +21,14 @@ nodes | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String | | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri | +| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | semmle.label | ... + ... | subpaths #select | 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 | | 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 | | 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 | +| 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 | +| 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 | diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java index 9e8ac93..50adb08 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java @@ -50,7 +50,45 @@ public void safe3(String userInput) throws IOException, URISyntaxException { } public void safe4() throws IOException, URISyntaxException { - // hardcoded, no untrusted input Desktop.getDesktop().browse(new URI("https://example.com")); } + + // rundll32 test cases + public void bad4_rundll32(HttpServletRequest request) throws IOException { + String url = request.getParameter("url"); + // Single string command with concatenation + Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url); + } + + public void bad5_rundll32(String userInput) throws IOException { + // Array-based command + String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", userInput }; + Runtime.getRuntime().exec(cmd); + } + + public void bad6_rundll32(HttpServletRequest request) throws IOException { + String url = request.getParameter("url"); + // ProcessBuilder with list + ProcessBuilder pb = new ProcessBuilder("rundll32", "url.dll,FileProtocolHandler", url); + pb.start(); + } + + public void safe5_rundll32(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + URI uri = new URI(url); + if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) { + Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url); + } + } + + public void safe6_rundll32(String userInput) throws IOException { + if (userInput.startsWith("https://") || userInput.startsWith("http://")) { + String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", userInput }; + Runtime.getRuntime().exec(cmd); + } + } + + public void safe7_rundll32() throws IOException { + Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler https://example.com"); + } } From b2d7c373a104f76443eb16f0592f9114cf075444 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Wed, 17 Dec 2025 14:32:41 -0500 Subject: [PATCH 3/6] add support for C++/Qt, and genericize shell execution models for all platforms --- .../PotentiallyUnguardedProtocolHandler.ql | 149 ++++++++++++++++++ ...tentiallyUnguardedProtocolHandler.expected | 4 + .../PotentiallyUnguardedProtocolHandler.qlref | 1 + .../test.cpp | 60 +++++++ .../PotentiallyUnguardedProtocolHandler.ql | 142 ++++++++++------- ...tentiallyUnguardedProtocolHandler.expected | 34 +++- .../PotentiallyUnguardedProtocolHandler.java | 45 ++++++ 7 files changed, 369 insertions(+), 66 deletions(-) create mode 100644 cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql create mode 100644 cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected create mode 100644 cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref create mode 100644 cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp diff --git a/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql b/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql new file mode 100644 index 0000000..4a7113f --- /dev/null +++ b/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql @@ -0,0 +1,149 @@ +/** + * @name Potentially unguarded protocol handler invocation + * @id tob/cpp/unguarded-protocol-handler + * @description Detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols + * @kind path-problem + * @tags security + * external/cwe/cwe-939 + * @precision medium + * @problem.severity warning + * @security-severity 6.5 + * @group security + */ + +import cpp +private import semmle.code.cpp.ir.dataflow.TaintTracking +private import semmle.code.cpp.security.FlowSources +private import semmle.code.cpp.security.CommandExecution + +/** + * Generic case: invoke protocol handling through OS's protocol handling utilities. This aligns with CVE-2022-43550. + */ +class ShellProtocolHandler extends SystemFunction { + ShellProtocolHandler() { + // Check if any calls to this function contain protocol handler invocations + exists(FunctionCall call | + call.getTarget() = this and + exists(Expr arg | + arg = call.getArgument(0).getAChild*() and + exists(StringLiteral sl | sl = arg or sl.getParent*() = arg | + sl.getValue() + .regexpMatch("(?i).*(rundll32.*url\\.dll.*FileProtocolHandler|xdg-open|\\bopen\\b).*") + ) + ) + ) + } + + string getHandlerType() { + exists(FunctionCall call, StringLiteral sl | + call.getTarget() = this and + sl.getParent*() = call.getArgument(0) and + ( + sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") and + result = "rundll32 url.dll,FileProtocolHandler" + or + sl.getValue().regexpMatch("(?i).*xdg-open.*") and + result = "xdg-open" + or + sl.getValue().regexpMatch("(?i).*\\bopen\\b.*") and + result = "open" + ) + ) + } +} + +/** + * Qt's QDesktopServices::openUrl method + */ +class QtProtocolHandler extends FunctionCall { + QtProtocolHandler() { this.getTarget().hasQualifiedName("QDesktopServices", "openUrl") } + + Expr getUrlArgument() { result = this.getArgument(0) } +} + +/** + * A sanitizer node that represents URL scheme validation + */ +class UrlSchemeValidationSanitizer extends DataFlow::Node { + UrlSchemeValidationSanitizer() { + exists(FunctionCall fc | + fc = this.asExpr() and + ( + // String comparison on the untrusted URL + fc.getTarget().getName() = + [ + "strcmp", "strncmp", "strcasecmp", "strncasecmp", "strstr", "strcasestr", "_stricmp", + "_strnicmp" + ] + or + // Qt QUrl::scheme() comparison - QUrl::scheme() returns QString + // Pattern: url.scheme() == "http" or url.scheme() == "https" + exists(FunctionCall schemeCall | + schemeCall.getTarget().hasQualifiedName("QUrl", "scheme") and + ( + // Direct comparison + fc.getTarget().hasName(["operator==", "operator!="]) and + fc.getAnArgument() = schemeCall + or + // QString comparison methods + fc = schemeCall and + exists(FunctionCall qstringCmp | + qstringCmp.getQualifier() = schemeCall and + qstringCmp.getTarget().hasQualifiedName("QString", ["compare", "operator=="]) + ) + ) + ) + or + // Qt QString startsWith check for direct URL strings + fc.getTarget().hasQualifiedName("QString", "startsWith") + ) + ) + } +} + +/** + * Configuration for tracking untrusted data to protocol handler invocations + */ +module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + predicate isSink(DataFlow::Node sink) { + // QDesktopServices::openUrl() + exists(QtProtocolHandler call | sink.asExpr() = call.getUrlArgument()) + or + // Shell protocol handlers (rundll32, xdg-open, open) via system()/popen()/exec*() + exists(FunctionCall call | + call.getTarget() instanceof ShellProtocolHandler and + sink.asExpr() = call.getArgument(0) + ) + } + + predicate isBarrier(DataFlow::Node node) { node instanceof UrlSchemeValidationSanitizer } +} + +module PotentiallyUnguardedProtocolHandlerFlow = + TaintTracking::Global; + +import PotentiallyUnguardedProtocolHandlerFlow::PathGraph + +from + PotentiallyUnguardedProtocolHandlerFlow::PathNode source, + PotentiallyUnguardedProtocolHandlerFlow::PathNode sink, FunctionCall call, string callType +where + PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and + ( + exists(QtProtocolHandler qtCall | + call = qtCall and + sink.getNode().asExpr() = qtCall.getUrlArgument() and + callType = "QDesktopServices::openUrl()" + ) + or + exists(ShellProtocolHandler shellFunc | + call.getTarget() = shellFunc and + sink.getNode().asExpr() = call.getArgument(0) and + callType = shellFunc.getHandlerType() + ) + ) +select call, source, sink, + callType + " is called with untrusted input from $@ without proper URL scheme validation.", + source.getNode(), "this source" diff --git a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected new file mode 100644 index 0000000..e217064 --- /dev/null +++ b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -0,0 +1,4 @@ +edges +nodes +subpaths +#select diff --git a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref new file mode 100644 index 0000000..de5b795 --- /dev/null +++ b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref @@ -0,0 +1 @@ +security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql diff --git a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp new file mode 100644 index 0000000..4579758 --- /dev/null +++ b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp @@ -0,0 +1,60 @@ +// Forward declarations - minimal to avoid system header dependencies +extern "C" { +int system(const char *); +int sprintf(char *, const char *, ...); +int strncmp(const char *, const char *, unsigned long); +} + +// Mock QString class +struct QString { + const char *data; + QString(const char *s) : data(s) {} + bool operator==(const char *other) const; +}; + +// Mock Qt-like class for QDesktopServices::openUrl +struct QUrl { + const char *url; + QUrl(const char *s) : url(s) {} + QString scheme() const; + bool startsWith(const char *prefix) const; +}; + +struct QDesktopServices { + static bool openUrl(const QUrl &url); +}; + +// Untrusted input sources +extern "C" char *getUserInput(); +extern "C" const char *getUrlParam(); + +// BAD: QDesktopServices::openUrl with untrusted input +void bad1_qt(const char *userUrl) { + QUrl url(userUrl); + QDesktopServices::openUrl(url); // BAD +} + +void bad2_qt() { + const char *input = getUrlParam(); + QUrl url(input); + QDesktopServices::openUrl(url); // BAD +} + +void safe1_qt() { + QUrl url("https://example.com"); + QDesktopServices::openUrl(url); // GOOD - no taint +} + +void safe2_qt(const char *userUrl) { + if (strncmp(userUrl, "https://", 8) == 0 || + strncmp(userUrl, "http://", 7) == 0) { + QUrl url(userUrl); + QDesktopServices::openUrl(url); // GOOD + } +} + +void safe3_qt(QUrl &url) { + if (url.scheme() == "https" || url.scheme() == "http") { + QDesktopServices::openUrl(url); // GOOD + } +} \ No newline at end of file diff --git a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql index b234905..33b3058 100644 --- a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql +++ b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql @@ -18,39 +18,24 @@ import semmle.code.java.controlflow.Guards import semmle.code.java.security.ExternalProcess /** - * A call to Desktop.browse() method. This is the platform-agnostic standard now. - * NOTE(alan): the URLRedirect query will likely also trigger due to a little imprecision there - */ -class DesktopBrowseCall extends MethodCall { - DesktopBrowseCall() { - this.getMethod().hasName("browse") and - this.getMethod().getDeclaringType().hasQualifiedName("java.awt", "Desktop") - } - - Expr getUrlArgument() { result = this.getArgument(0) } -} - -/** - * Legacy Windows-specific command execution for protocol handling, based on https://imagej.net/ij/source/ij/plugin/BrowserLauncher.java and - * seen in CVE-2022-43550. We match on the Windows shell command, since it more distinct and pervasive. + * Generic case: invoke protocol handling through OS's protocol handling utilities. This aligns with CVE-2022-43550. */ -class LegacyWindowsProtocolHandlerCall extends ArgumentToExec { - LegacyWindowsProtocolHandlerCall() { - // Single string: "rundll32 url.dll,FileProtocolHandler " + url +class ShellProtocolHandler extends ArgumentToExec { + ShellProtocolHandler() { + // Single string: "rundll32 url.dll,FileProtocolHandler " + url or "xdg-open " + url this instanceof StringArgumentToExec and exists(StringLiteral sl | sl.getParent*() = this and - sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") + sl.getValue() + .regexpMatch("(?i).*(rundll32.*url\\.dll.*FileProtocolHandler|xdg-open|\\bopen\\b).*") ) or - // Array: {"rundll32", "url.dll,FileProtocolHandler", url} + // Array: {"rundll32", "url.dll,FileProtocolHandler", url} or {"xdg-open", url} this.getType().(Array).getElementType() instanceof TypeString and - exists(ArrayInit init, StringLiteral rundll, StringLiteral urldll | + exists(ArrayInit init, StringLiteral handler | init = this.(ArrayCreationExpr).getInit() and - rundll = init.getAnInit() and - urldll = init.getAnInit() and - rundll.getValue().regexpMatch("(?i)rundll32(\\.exe)?") and - urldll.getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler") + handler = init.getAnInit() and + handler.getValue().regexpMatch("(?i)(rundll32(\\.exe)?|xdg-open|open|/usr/bin/open)") ) } @@ -58,41 +43,84 @@ class LegacyWindowsProtocolHandlerCall extends ArgumentToExec { // For single string exec, the URL is tainted into the concatenated string result = this and this instanceof StringArgumentToExec or - // For arrays, find elements after "url.dll,FileProtocolHandler" + // For arrays with rundll32, find elements after "url.dll,FileProtocolHandler" exists(ArrayInit init, int urlIdx, int dllIdx | init = this.(ArrayCreationExpr).getInit() and result = init.getInit(urlIdx) and - init.getInit(dllIdx).(StringLiteral).getValue().regexpMatch("(?i)url\\.dll.*FileProtocolHandler") and + init.getInit(dllIdx) + .(StringLiteral) + .getValue() + .regexpMatch("(?i)url\\.dll.*FileProtocolHandler") and urlIdx > dllIdx ) + or + // For arrays with xdg-open/open, find elements after the handler + exists(ArrayInit init, int urlIdx, int handlerIdx | + init = this.(ArrayCreationExpr).getInit() and + result = init.getInit(urlIdx) and + init.getInit(handlerIdx) + .(StringLiteral) + .getValue() + .regexpMatch("(?i)(xdg-open|open|/usr/bin/open)") and + urlIdx > handlerIdx + ) + } + + string getHandlerType() { + exists(StringLiteral sl | + sl.getParent*() = this and + ( + sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") and + result = "rundll32 url.dll,FileProtocolHandler" + or + sl.getValue().regexpMatch("(?i).*xdg-open.*") and + result = "xdg-open" + or + sl.getValue().regexpMatch("(?i).*\\bopen\\b.*") and + result = "open" + ) + ) } } /** - * A guard that checks the URL scheme/protocol signifying sanitization before launch + * A call to Desktop.browse() method. This is the platform-agnostic standard now. + * NOTE(alan): the URLRedirect query will likely also trigger due to a little imprecision there */ -predicate isUrlSchemeCheck(Guard g, Expr e, boolean branch) { - exists(MethodCall mc | - mc = g and - e = mc.getQualifier*() and - branch = true and - ( - // getScheme() or getProtocol() check - mc.getMethod().hasName(["equals", "equalsIgnoreCase", "startsWith", "matches"]) and - exists(MethodCall getScheme | - getScheme.getParent*() = mc and - getScheme.getMethod().hasName(["getScheme", "getProtocol"]) +class DesktopBrowseProtocolHandler extends MethodCall { + DesktopBrowseProtocolHandler() { + this.getMethod().hasName("browse") and + this.getMethod().getDeclaringType().hasQualifiedName("java.awt", "Desktop") + } + + Expr getUrlArgument() { result = this.getArgument(0) } +} + +/** + * A sanitizer node that represents URL scheme validation + */ +class UrlSchemeValidationSanitizer extends DataFlow::Node { + UrlSchemeValidationSanitizer() { + exists(MethodCall mc | + mc = this.asExpr() and + ( + // String comparison on the untrusted URL + mc.getMethod().hasName(["equals", "equalsIgnoreCase", "startsWith", "matches"]) and + exists(MethodCall getScheme | + getScheme.getParent*() = mc and + getScheme.getMethod().hasName(["getScheme", "getProtocol"]) + ) + or + // URL string contains check like: url.contains("://") + mc.getMethod().hasName(["contains", "startsWith", "matches", "indexOf"]) and + exists(StringLiteral sl | sl = mc.getAnArgument() | sl.getValue().regexpMatch(".*://.*")) + or + // Pattern matching on the string representation + mc.getMethod().hasName(["matches", "find"]) and + mc.getQualifier().getType().(RefType).hasQualifiedName("java.util.regex", "Matcher") ) - or - // URL string contains check like: url.contains("http://") or url.startsWith("https://") - mc.getMethod().hasName(["contains", "startsWith", "matches", "indexOf"]) and - exists(StringLiteral sl | sl = mc.getAnArgument() | sl.getValue().regexpMatch(".*://.*")) - or - // Pattern matching on the string representation - mc.getMethod().hasName(["matches", "find"]) and - mc.getQualifier().getType().(RefType).hasQualifiedName("java.util.regex", "Matcher") ) - ) + } } /** @@ -102,14 +130,12 @@ module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { - exists(DesktopBrowseCall call | sink.asExpr() = call.getUrlArgument()) + exists(DesktopBrowseProtocolHandler call | sink.asExpr() = call.getUrlArgument()) or - exists(LegacyWindowsProtocolHandlerCall call | sink.asExpr() = call.getUrlArgument()) + exists(ShellProtocolHandler call | sink.asExpr() = call.getUrlArgument()) } - predicate isBarrier(DataFlow::Node node) { - node = DataFlow::BarrierGuard::getABarrierNode() - } + predicate isBarrier(DataFlow::Node node) { node instanceof UrlSchemeValidationSanitizer } } module PotentiallyUnguardedProtocolHandlerFlow = @@ -123,16 +149,16 @@ from where PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and ( - exists(DesktopBrowseCall dbc | + exists(DesktopBrowseProtocolHandler dbc | call = dbc and sink.getNode().asExpr() = dbc.getUrlArgument() and callType = "Desktop.browse()" ) or - exists(LegacyWindowsProtocolHandlerCall whc | - call = whc and - sink.getNode().asExpr() = whc.getUrlArgument() and - callType = "rundll32 url.dll,FileProtocolHandler" + exists(ShellProtocolHandler shc | + call = shc and + sink.getNode().asExpr() = shc.getUrlArgument() and + callType = shc.getHandlerType() ) ) select call, source, sink, diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected index d8a7dac..d621e3a 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -7,8 +7,12 @@ edges | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 | | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 | -| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | +| PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | nodes | PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) | @@ -21,14 +25,28 @@ nodes | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String | | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri | -| PotentiallyUnguardedProtocolHandler.java:59:22:59:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:61:35:61:79 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:78:22:78:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:83:39:83:83 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | +| PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | semmle.label | ... + ... | subpaths #select | 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 | | 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 | | 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 | -| 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 | -| 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 | +| PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | xdg-open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | xdg-open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) | this source | diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java index 50adb08..4642b5f 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java @@ -91,4 +91,49 @@ public void safe6_rundll32(String userInput) throws IOException { public void safe7_rundll32() throws IOException { Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler https://example.com"); } + + // xdg-open test cases (Linux) + public void bad7_xdgopen(HttpServletRequest request) throws IOException { + String url = request.getParameter("url"); + Runtime.getRuntime().exec("xdg-open " + url); + } + + public void bad8_xdgopen(String userInput) throws IOException { + String[] cmd = { "xdg-open", userInput }; + Runtime.getRuntime().exec(cmd); + } + + public void safe8_xdgopen(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + URI uri = new URI(url); + if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) { + Runtime.getRuntime().exec("xdg-open " + url); + } + } + + public void safe9_xdgopen() throws IOException { + Runtime.getRuntime().exec("xdg-open https://example.com"); + } + + // open test cases (macOS) + public void bad9_open(HttpServletRequest request) throws IOException { + String url = request.getParameter("url"); + Runtime.getRuntime().exec("open " + url); + } + + public void bad10_open(String userInput) throws IOException { + ProcessBuilder pb = new ProcessBuilder("open", userInput); + pb.start(); + } + + public void safe10_open(HttpServletRequest request) throws IOException, URISyntaxException { + String url = request.getParameter("url"); + if (url.startsWith("https://") || url.startsWith("http://")) { + Runtime.getRuntime().exec("/usr/bin/open " + url); + } + } + + public void safe11_open() throws IOException { + Runtime.getRuntime().exec("open https://example.com"); + } } From 7efd379b71c15ff50a38996ad77d49e080a522e0 Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Fri, 2 Jan 2026 12:49:08 -0500 Subject: [PATCH 4/6] fix java tests --- ...tentiallyUnguardedProtocolHandler.expected | 32 ++--------- .../PotentiallyUnguardedProtocolHandler.java | 53 +++---------------- 2 files changed, 9 insertions(+), 76 deletions(-) diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected index d621e3a..14f3d34 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -5,14 +5,8 @@ edges | PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:23:41:23:43 | uri | provenance | Sink:MaD:43969 | | PotentiallyUnguardedProtocolHandler.java:21:27:21:29 | url : String | PotentiallyUnguardedProtocolHandler.java:21:19:21:30 | new URI(...) : URI | provenance | MaD:44428 | | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | provenance | Src:MaD:46190 | -| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | provenance | Sink:MaD:43969 | +| PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | PotentiallyUnguardedProtocolHandler.java:38:41:38:43 | uri | provenance | Sink:MaD:43969 | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | provenance | MaD:44428 | -| PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | -| PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | provenance | Src:MaD:46190 Sink:MaD:44131 | nodes | PotentiallyUnguardedProtocolHandler.java:10:22:10:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | | PotentiallyUnguardedProtocolHandler.java:11:37:11:48 | new URI(...) | semmle.label | new URI(...) | @@ -24,29 +18,9 @@ nodes | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | | PotentiallyUnguardedProtocolHandler.java:35:19:35:30 | new URI(...) : URI | semmle.label | new URI(...) : URI | | PotentiallyUnguardedProtocolHandler.java:35:27:35:29 | url : String | semmle.label | url : String | -| PotentiallyUnguardedProtocolHandler.java:39:41:39:43 | uri | semmle.label | uri | -| PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | semmle.label | ... + ... | -| PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | semmle.label | getParameter(...) : String | -| PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | semmle.label | ... + ... | +| PotentiallyUnguardedProtocolHandler.java:38:41:38:43 | uri | semmle.label | uri | subpaths #select | 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 | | 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 | -| 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 | -| PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:60:35:60:79 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:58:22:58:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:80:39:80:83 | ... + ... | rundll32 url.dll,FileProtocolHandler is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:77:22:77:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:98:35:98:51 | ... + ... | xdg-open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:97:22:97:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:110:39:110:55 | ... + ... | xdg-open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:107:22:107:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:121:35:121:47 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:120:22:120:48 | getParameter(...) | this source | -| PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:132:39:132:60 | ... + ... | open is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:130:22:130:48 | getParameter(...) | this source | +| PotentiallyUnguardedProtocolHandler.java:38:13:38:44 | browse(...) | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) : String | PotentiallyUnguardedProtocolHandler.java:38:41:38:43 | uri | Desktop.browse() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.java:34:22:34:48 | getParameter(...) | this source | diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java index 4642b5f..37436d3 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.java @@ -33,7 +33,6 @@ public void safe2(String userInput) throws IOException, URISyntaxException { public void bad3(HttpServletRequest request) throws IOException, URISyntaxException { String url = request.getParameter("url"); URI uri = new URI(url); - // Weak check - only checks if scheme exists, not what it is if (uri.getScheme() != null) { Desktop.getDesktop().browse(uri); @@ -43,7 +42,6 @@ public void bad3(HttpServletRequest request) throws IOException, URISyntaxExcept public void safe3(String userInput) throws IOException, URISyntaxException { URI uri = new URI(userInput); String scheme = uri.getScheme(); - if (scheme != null && (scheme.equalsIgnoreCase("http") || scheme.equalsIgnoreCase("https"))) { Desktop.getDesktop().browse(uri); } @@ -53,22 +51,13 @@ public void safe4() throws IOException, URISyntaxException { Desktop.getDesktop().browse(new URI("https://example.com")); } - // rundll32 test cases - public void bad4_rundll32(HttpServletRequest request) throws IOException { - String url = request.getParameter("url"); - // Single string command with concatenation - Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url); - } - - public void bad5_rundll32(String userInput) throws IOException { - // Array-based command + public void bad4_rundll32(String userInput) throws IOException { String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", userInput }; Runtime.getRuntime().exec(cmd); } public void bad6_rundll32(HttpServletRequest request) throws IOException { String url = request.getParameter("url"); - // ProcessBuilder with list ProcessBuilder pb = new ProcessBuilder("rundll32", "url.dll,FileProtocolHandler", url); pb.start(); } @@ -77,7 +66,8 @@ public void safe5_rundll32(HttpServletRequest request) throws IOException, URISy String url = request.getParameter("url"); URI uri = new URI(url); if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) { - Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler " + url); + String[] cmd = { "rundll32", "url.dll,FileProtocolHandler", url }; + Runtime.getRuntime().exec(cmd); } } @@ -92,13 +82,7 @@ public void safe7_rundll32() throws IOException { Runtime.getRuntime().exec("rundll32 url.dll,FileProtocolHandler https://example.com"); } - // xdg-open test cases (Linux) - public void bad7_xdgopen(HttpServletRequest request) throws IOException { - String url = request.getParameter("url"); - Runtime.getRuntime().exec("xdg-open " + url); - } - - public void bad8_xdgopen(String userInput) throws IOException { + public void bad7_xdgopen(String userInput) throws IOException { String[] cmd = { "xdg-open", userInput }; Runtime.getRuntime().exec(cmd); } @@ -107,33 +91,8 @@ public void safe8_xdgopen(HttpServletRequest request) throws IOException, URISyn String url = request.getParameter("url"); URI uri = new URI(url); if (uri.getScheme().equals("https") || uri.getScheme().equals("http")) { - Runtime.getRuntime().exec("xdg-open " + url); - } - } - - public void safe9_xdgopen() throws IOException { - Runtime.getRuntime().exec("xdg-open https://example.com"); - } - - // open test cases (macOS) - public void bad9_open(HttpServletRequest request) throws IOException { - String url = request.getParameter("url"); - Runtime.getRuntime().exec("open " + url); - } - - public void bad10_open(String userInput) throws IOException { - ProcessBuilder pb = new ProcessBuilder("open", userInput); - pb.start(); - } - - public void safe10_open(HttpServletRequest request) throws IOException, URISyntaxException { - String url = request.getParameter("url"); - if (url.startsWith("https://") || url.startsWith("http://")) { - Runtime.getRuntime().exec("/usr/bin/open " + url); + String[] cmd = { "xdg-open", url }; + Runtime.getRuntime().exec(cmd); } } - - public void safe11_open() throws IOException { - Runtime.getRuntime().exec("open https://example.com"); - } } From 4d28876987c6283a924eafb0980db51cbc7ac45b Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Wed, 14 Jan 2026 16:28:42 -0500 Subject: [PATCH 5/6] Add docs and fix CI/CD issue --- README.md | 5 +- .../PotentiallyUnguardedProtocolHandler.md | 87 ++++++++++++++++ .../PotentiallyUnguardedProtocolHandler.md | 99 +++++++++++++++++++ .../PotentiallyUnguardedProtocolHandler.qlref | 2 +- 4 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 cpp/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md create mode 100644 java/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md diff --git a/README.md b/README.md index 8488b85..402f6ba 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,8 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif - |[Invalid string size passed to string manipulation function](./cpp/src/docs/security/CStrnFinder/CStrnFinder.md)|Finds calls to functions that take as input a string and its size as separate arguments (e.g., `strncmp`, `strncat`, ...) and the size argument is wrong|error|low| |[Iterator invalidation](./cpp/src/docs/security/IteratorInvalidation/IteratorInvalidation.md)|Modifying a container while iterating over it can invalidate iterators, leading to undefined behavior.|warning|medium| |[Missing null terminator](./cpp/src/docs/security/NoNullTerminator/NoNullTerminator.md)|This query finds incorrectly initialized strings that are passed to functions expecting null-byte-terminated strings|error|high| +|[Potentially unguarded protocol handler invocation](./cpp/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md)|Detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols|warning|medium| +|[Unsafe implicit integer conversion](./cpp/src/docs/security/UnsafeImplicitConversions/UnsafeImplicitConversions.md)|Finds implicit integer casts that may overflow or be truncated, with false positive reduction via Value Range Analysis|warning|low| ### Go @@ -77,7 +79,8 @@ codeql database analyze database.db --format=sarif-latest --output=./tob.sarif - | Name | Description | Severity | Precision | | --- | ----------- | :----: | :--------: | -|[Recursive functions](./java-kotlin/src/docs/security/Recursion/Recursion.md)|Detects possibly unbounded recursive calls|warning|low| +|[Potentially unguarded protocol handler invocation](./java/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md)|Detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols|warning|medium| +|[Recursive functions](./java-kotlin/src/docs/security/Recursion/Recursion.md)|Detects recursive calls|warning|low| ## Query suites diff --git a/cpp/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md b/cpp/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md new file mode 100644 index 0000000..5e56eef --- /dev/null +++ b/cpp/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md @@ -0,0 +1,87 @@ +# Potentially unguarded protocol handler invocation + +This query detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols. This vulnerability is related to CWE-939 (Improper Authorization in Handler for Custom URL Scheme) and aligns with CVE-2022-43550. + +When applications invoke protocol handlers (like `rundll32 url.dll,FileProtocolHandler` on Windows, `xdg-open` on Linux, `open` on macOS, or Qt's `QDesktopServices::openUrl()`), untrusted URLs can potentially trigger dangerous protocols such as `file://`, `smb://`, or other custom handlers that may lead to unauthorized file access, command execution, or other security issues. + +## Detected patterns + +The query identifies several common protocol handler invocation patterns: + +- **Windows**: `rundll32 url.dll,FileProtocolHandler` via system calls +- **Linux**: `xdg-open` via system calls +- **macOS**: `open` command via system calls +- **Qt applications**: `QDesktopServices::openUrl()` + +## Recommendation + +Always validate URL schemes before passing them to protocol handlers. Only allow safe protocols like `http://` and `https://`. Reject or sanitize URLs containing potentially dangerous protocols. + +## Example + +The following vulnerable code passes untrusted input directly to a protocol handler: + +```cpp +#include +#include + +void openUserUrl(const QString& userInput) { + // VULNERABLE: No validation of the URL scheme + QDesktopServices::openUrl(QUrl(userInput)); +} +``` + +An attacker could provide a URL like `file:///etc/passwd` or `smb://attacker-server/share` to access unauthorized resources. + +The corrected version validates the URL scheme before opening: + +```cpp +#include +#include + +void openUserUrl(const QString& userInput) { + QUrl url(userInput); + QString scheme = url.scheme().toLower(); + + // Only allow safe protocols + if (scheme == "http" || scheme == "https") { + QDesktopServices::openUrl(url); + } else { + // Log error or show warning to user + qWarning() << "Rejected unsafe URL scheme:" << scheme; + } +} +``` + +For system command invocations: + +```cpp +void openUserUrlViaShell(const char* userInput) { + // VULNERABLE: Untrusted input passed to xdg-open + char cmd[512]; + snprintf(cmd, sizeof(cmd), "xdg-open '%s'", userInput); + system(cmd); +} +``` + +Should be corrected to validate the scheme: + +```cpp +bool isValidScheme(const char* url) { + return (strncasecmp(url, "http://", 7) == 0 || + strncasecmp(url, "https://", 8) == 0); +} + +void openUserUrlViaShell(const char* userInput) { + if (isValidScheme(userInput)) { + char cmd[512]; + snprintf(cmd, sizeof(cmd), "xdg-open '%s'", userInput); + system(cmd); + } +} +``` + +## References + +- [CWE-939: Improper Authorization in Handler for Custom URL Scheme](https://cwe.mitre.org/data/definitions/939.html) +- [CVE-2022-43550: USB Creator has insufficiently protected credentials](https://ubuntu.com/security/CVE-2022-43550) diff --git a/java/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md b/java/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md new file mode 100644 index 0000000..445201d --- /dev/null +++ b/java/src/docs/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.md @@ -0,0 +1,99 @@ +# Potentially unguarded protocol handler invocation + +This query detects calls to URL protocol handlers with untrusted input that may not be properly validated for dangerous protocols. This vulnerability is related to CWE-939 (Improper Authorization in Handler for Custom URL Scheme) and aligns with CVE-2022-43550. + +When Java applications invoke protocol handlers (like `Desktop.browse()`, `Runtime.exec()` with `xdg-open`/`open`, or `rundll32 url.dll,FileProtocolHandler` on Windows), untrusted URLs can potentially trigger dangerous protocols such as `file://`, `smb://`, or other custom handlers that may lead to unauthorized file access, command execution, or other security issues. + +## Detected patterns + +The query identifies several common protocol handler invocation patterns: + +- **Java AWT**: `Desktop.browse(URI)` - the platform-agnostic standard +- **Windows**: `Runtime.exec()` with `rundll32 url.dll,FileProtocolHandler` +- **Linux**: `Runtime.exec()` with `xdg-open` +- **macOS**: `Runtime.exec()` with `open` command + +## Recommendation + +Always validate URL schemes before passing them to protocol handlers. Only allow safe protocols like `http://` and `https://`. Reject or sanitize URLs containing potentially dangerous protocols. + +## Example + +The following vulnerable code passes untrusted input directly to a protocol handler: + +```java +import java.awt.Desktop; +import java.net.URI; + +public class UrlOpener { + public void openUserUrl(String userInput) throws Exception { + // VULNERABLE: No validation of the URL scheme + Desktop.getDesktop().browse(new URI(userInput)); + } +} +``` + +An attacker could provide a URL like `file:///etc/passwd` or `smb://attacker-server/share` to access unauthorized resources. + +The corrected version validates the URL scheme before opening: + +```java +import java.awt.Desktop; +import java.net.URI; + +public class UrlOpener { + public void openUserUrl(String userInput) throws Exception { + URI uri = new URI(userInput); + String scheme = uri.getScheme(); + + // Only allow safe protocols + if ("http".equalsIgnoreCase(scheme) || "https".equalsIgnoreCase(scheme)) { + Desktop.getDesktop().browse(uri); + } else { + throw new SecurityException("Rejected unsafe URL scheme: " + scheme); + } + } +} +``` + +For system command invocations: + +```java +public class UrlOpener { + public void openUserUrlViaShell(String userInput) throws Exception { + // VULNERABLE: Untrusted input passed to xdg-open + Runtime.getRuntime().exec(new String[]{"xdg-open", userInput}); + } +} +``` + +Should be corrected to validate the scheme: + +```java +import java.net.URI; + +public class UrlOpener { + private boolean isValidScheme(String url) { + try { + URI uri = new URI(url); + String scheme = uri.getScheme(); + return "http".equalsIgnoreCase(scheme) || "https".equalsIgnoreCase(scheme); + } catch (Exception e) { + return false; + } + } + + public void openUserUrlViaShell(String userInput) throws Exception { + if (isValidScheme(userInput)) { + Runtime.getRuntime().exec(new String[]{"xdg-open", userInput}); + } else { + throw new SecurityException("Invalid or unsafe URL scheme"); + } + } +} +``` + +## References + +- [CWE-939: Improper Authorization in Handler for Custom URL Scheme](https://cwe.mitre.org/data/definitions/939.html) +- [CVE-2022-43550: USB Creator has insufficiently protected credentials](https://ubuntu.com/security/CVE-2022-43550) diff --git a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref index bc3c1da..de5b795 100644 --- a/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref +++ b/java/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qlref @@ -1 +1 @@ -security/PotentiallyUnguardedProtocolhandler/PotentiallyUnguardedProtocolhandler.ql +security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql From e0d42d1e1918800abcd4c775911181286064510e Mon Sep 17 00:00:00 2001 From: alan <17283637+ex0dus-0x@users.noreply.github.com> Date: Mon, 6 Apr 2026 14:00:12 -0400 Subject: [PATCH 6/6] Fix up tests, and enhance query with better isBarrier --- .../PotentiallyUnguardedProtocolHandler.ql | 153 ++++++++---------- cpp/test/include/libc/stdint.h | 2 + cpp/test/include/libc/stdlib.h | 2 + cpp/test/include/libc/string_stubs.h | 2 + cpp/test/include/libc/unistd.h | 2 + ...tentiallyUnguardedProtocolHandler.expected | 8 + .../test.cpp | 60 ------- .../PotentiallyUnguardedProtocolHandler.qhelp | 9 ++ 8 files changed, 89 insertions(+), 149 deletions(-) delete mode 100644 cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp diff --git a/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql b/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql index 4a7113f..60fd383 100644 --- a/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql +++ b/cpp/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.ql @@ -15,110 +15,92 @@ import cpp private import semmle.code.cpp.ir.dataflow.TaintTracking private import semmle.code.cpp.security.FlowSources private import semmle.code.cpp.security.CommandExecution +private import semmle.code.cpp.controlflow.IRGuards /** - * Generic case: invoke protocol handling through OS's protocol handling utilities. This aligns with CVE-2022-43550. + * Holds when `call` invokes a system function (system, popen, exec*) with a command string + * that contains a URL protocol handler, `sink` is the argument carrying the URL, and + * `handlerType` describes which handler is invoked. */ -class ShellProtocolHandler extends SystemFunction { - ShellProtocolHandler() { - // Check if any calls to this function contain protocol handler invocations - exists(FunctionCall call | - call.getTarget() = this and - exists(Expr arg | - arg = call.getArgument(0).getAChild*() and - exists(StringLiteral sl | sl = arg or sl.getParent*() = arg | - sl.getValue() - .regexpMatch("(?i).*(rundll32.*url\\.dll.*FileProtocolHandler|xdg-open|\\bopen\\b).*") - ) - ) +predicate isShellProtocolHandlerSink(FunctionCall call, Expr sink, string handlerType) { + call.getTarget() instanceof SystemFunction and + exists(StringLiteral sl | + sl.getParent*() = call.getArgument(0) and + ( + sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") and + handlerType = "rundll32 url.dll,FileProtocolHandler" + or + sl.getValue().regexpMatch("(?i).*xdg-open.*") and + handlerType = "xdg-open" + or + sl.getValue().regexpMatch("(?i).*\\bopen\\b.*") and + handlerType = "open" ) - } - - string getHandlerType() { - exists(FunctionCall call, StringLiteral sl | - call.getTarget() = this and - sl.getParent*() = call.getArgument(0) and - ( - sl.getValue().regexpMatch("(?i).*rundll32.*url\\.dll.*FileProtocolHandler.*") and - result = "rundll32 url.dll,FileProtocolHandler" - or - sl.getValue().regexpMatch("(?i).*xdg-open.*") and - result = "xdg-open" - or - sl.getValue().regexpMatch("(?i).*\\bopen\\b.*") and - result = "open" - ) - ) - } + ) and + sink = call.getArgument(0) } /** - * Qt's QDesktopServices::openUrl method + * Holds when `call` invokes QDesktopServices::openUrl and `sink` is the URL argument. */ -class QtProtocolHandler extends FunctionCall { - QtProtocolHandler() { this.getTarget().hasQualifiedName("QDesktopServices", "openUrl") } +predicate isQtProtocolHandlerSink(FunctionCall call, Expr sink) { + call.getTarget().hasQualifiedName("", "QDesktopServices", "openUrl") and + sink = call.getArgument(0) +} - Expr getUrlArgument() { result = this.getArgument(0) } +/** + * Gets the sink expression for a given DataFlow::Node matching either Qt or shell handler sinks. + */ +Expr getSinkExpr(DataFlow::Node sink) { + result = sink.asExpr() or + result = sink.asIndirectExpr() } /** - * A sanitizer node that represents URL scheme validation + * Holds when `g` is a guard condition that validates the URL scheme of expression `e`. + * Flow is considered safe on the `branch` edge. */ -class UrlSchemeValidationSanitizer extends DataFlow::Node { - UrlSchemeValidationSanitizer() { - exists(FunctionCall fc | - fc = this.asExpr() and - ( - // String comparison on the untrusted URL - fc.getTarget().getName() = - [ - "strcmp", "strncmp", "strcasecmp", "strncasecmp", "strstr", "strcasestr", "_stricmp", - "_strnicmp" - ] - or - // Qt QUrl::scheme() comparison - QUrl::scheme() returns QString - // Pattern: url.scheme() == "http" or url.scheme() == "https" - exists(FunctionCall schemeCall | - schemeCall.getTarget().hasQualifiedName("QUrl", "scheme") and - ( - // Direct comparison - fc.getTarget().hasName(["operator==", "operator!="]) and - fc.getAnArgument() = schemeCall - or - // QString comparison methods - fc = schemeCall and - exists(FunctionCall qstringCmp | - qstringCmp.getQualifier() = schemeCall and - qstringCmp.getTarget().hasQualifiedName("QString", ["compare", "operator=="]) - ) - ) - ) - or - // Qt QString startsWith check for direct URL strings - fc.getTarget().hasQualifiedName("QString", "startsWith") - ) +predicate urlSchemeGuardChecks(IRGuardCondition g, Expr e, boolean branch) { + branch = [true, false] and + exists(FunctionCall fc | g.getUnconvertedResultExpression().getAChild*() = fc | + // C string comparison on the URL (strcmp, strncmp, etc.) + fc.getTarget().getName() = + [ + "strcmp", "strncmp", "strcasecmp", "strncasecmp", "strstr", "strcasestr", "_stricmp", + "_strnicmp" + ] and + e = fc.getAnArgument() + or + // Qt QString::startsWith check + fc.getTarget().hasQualifiedName("", "QString", "startsWith") and + e = fc.getQualifier() + or + // Qt QUrl::scheme() comparison + exists(FunctionCall schemeCall | + schemeCall.getTarget().hasQualifiedName("", "QUrl", "scheme") and + fc.getAnArgument() = schemeCall and + e = schemeCall.getQualifier() ) - } + ) } /** - * Configuration for tracking untrusted data to protocol handler invocations + * Configuration for tracking untrusted data to protocol handler invocations. */ module PotentiallyUnguardedProtocolHandlerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } predicate isSink(DataFlow::Node sink) { - // QDesktopServices::openUrl() - exists(QtProtocolHandler call | sink.asExpr() = call.getUrlArgument()) + isQtProtocolHandlerSink(_, getSinkExpr(sink)) or - // Shell protocol handlers (rundll32, xdg-open, open) via system()/popen()/exec*() - exists(FunctionCall call | - call.getTarget() instanceof ShellProtocolHandler and - sink.asExpr() = call.getArgument(0) - ) + isShellProtocolHandlerSink(_, getSinkExpr(sink), _) } - predicate isBarrier(DataFlow::Node node) { node instanceof UrlSchemeValidationSanitizer } + predicate isBarrier(DataFlow::Node node) { + node = DataFlow::BarrierGuard::getABarrierNode() + or + node = DataFlow::BarrierGuard::getAnIndirectBarrierNode() + } } module PotentiallyUnguardedProtocolHandlerFlow = @@ -132,17 +114,10 @@ from where PotentiallyUnguardedProtocolHandlerFlow::flowPath(source, sink) and ( - exists(QtProtocolHandler qtCall | - call = qtCall and - sink.getNode().asExpr() = qtCall.getUrlArgument() and - callType = "QDesktopServices::openUrl()" - ) + isQtProtocolHandlerSink(call, getSinkExpr(sink.getNode())) and + callType = "QDesktopServices::openUrl()" or - exists(ShellProtocolHandler shellFunc | - call.getTarget() = shellFunc and - sink.getNode().asExpr() = call.getArgument(0) and - callType = shellFunc.getHandlerType() - ) + isShellProtocolHandlerSink(call, getSinkExpr(sink.getNode()), callType) ) select call, source, sink, callType + " is called with untrusted input from $@ without proper URL scheme validation.", diff --git a/cpp/test/include/libc/stdint.h b/cpp/test/include/libc/stdint.h index d02afcd..12c4a23 100644 --- a/cpp/test/include/libc/stdint.h +++ b/cpp/test/include/libc/stdint.h @@ -6,6 +6,8 @@ typedef unsigned char uint8_t; typedef unsigned short uint16_t; typedef unsigned int uint32_t; +typedef unsigned long size_t; +typedef long ssize_t; #endif #else // --- else USE_HEADERS diff --git a/cpp/test/include/libc/stdlib.h b/cpp/test/include/libc/stdlib.h index 7f28000..e35eb79 100644 --- a/cpp/test/include/libc/stdlib.h +++ b/cpp/test/include/libc/stdlib.h @@ -7,6 +7,8 @@ extern "C" { #endif +extern int system(const char *); + extern void *reallocf(void *, unsigned long); int rand(void) { diff --git a/cpp/test/include/libc/string_stubs.h b/cpp/test/include/libc/string_stubs.h index d589139..c9ff6b1 100644 --- a/cpp/test/include/libc/string_stubs.h +++ b/cpp/test/include/libc/string_stubs.h @@ -28,6 +28,8 @@ extern int wprintf(const wchar_t * format, ...); extern wchar_t* wcscpy(wchar_t * s1, const wchar_t * s2); extern void perror(const char *s); extern int puts(const char *s); +extern int strcmp(const char *, const char *); +extern int strncmp(const char *, const char *, size_t); extern void openlog(const char*, int, int); extern void syslog(int, const char*, ...); diff --git a/cpp/test/include/libc/unistd.h b/cpp/test/include/libc/unistd.h index 7163a23..16f2352 100644 --- a/cpp/test/include/libc/unistd.h +++ b/cpp/test/include/libc/unistd.h @@ -7,6 +7,8 @@ extern "C" { #endif +ssize_t read(int, void *, size_t); + void _exit(int); #ifdef __cplusplus diff --git a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected index e217064..672f879 100644 --- a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected +++ b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.expected @@ -1,4 +1,12 @@ edges +| PotentiallyUnguardedProtocolHandler.cpp:27:11:27:13 | read output argument | PotentiallyUnguardedProtocolHandler.cpp:28:29:28:31 | *buf | provenance | | +| PotentiallyUnguardedProtocolHandler.cpp:39:11:39:13 | read output argument | PotentiallyUnguardedProtocolHandler.cpp:42:31:42:33 | *buf | provenance | | nodes +| PotentiallyUnguardedProtocolHandler.cpp:27:11:27:13 | read output argument | semmle.label | read output argument | +| PotentiallyUnguardedProtocolHandler.cpp:28:29:28:31 | *buf | semmle.label | *buf | +| PotentiallyUnguardedProtocolHandler.cpp:39:11:39:13 | read output argument | semmle.label | read output argument | +| PotentiallyUnguardedProtocolHandler.cpp:42:31:42:33 | *buf | semmle.label | *buf | subpaths #select +| PotentiallyUnguardedProtocolHandler.cpp:28:3:28:27 | call to openUrl | PotentiallyUnguardedProtocolHandler.cpp:27:11:27:13 | read output argument | PotentiallyUnguardedProtocolHandler.cpp:28:29:28:31 | *buf | QDesktopServices::openUrl() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.cpp:27:11:27:13 | read output argument | this source | +| PotentiallyUnguardedProtocolHandler.cpp:42:5:42:29 | call to openUrl | PotentiallyUnguardedProtocolHandler.cpp:39:11:39:13 | read output argument | PotentiallyUnguardedProtocolHandler.cpp:42:31:42:33 | *buf | QDesktopServices::openUrl() is called with untrusted input from $@ without proper URL scheme validation. | PotentiallyUnguardedProtocolHandler.cpp:39:11:39:13 | read output argument | this source | diff --git a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp b/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp deleted file mode 100644 index 4579758..0000000 --- a/cpp/test/query-tests/security/PotentiallyUnguardedProtocolHandler/test.cpp +++ /dev/null @@ -1,60 +0,0 @@ -// Forward declarations - minimal to avoid system header dependencies -extern "C" { -int system(const char *); -int sprintf(char *, const char *, ...); -int strncmp(const char *, const char *, unsigned long); -} - -// Mock QString class -struct QString { - const char *data; - QString(const char *s) : data(s) {} - bool operator==(const char *other) const; -}; - -// Mock Qt-like class for QDesktopServices::openUrl -struct QUrl { - const char *url; - QUrl(const char *s) : url(s) {} - QString scheme() const; - bool startsWith(const char *prefix) const; -}; - -struct QDesktopServices { - static bool openUrl(const QUrl &url); -}; - -// Untrusted input sources -extern "C" char *getUserInput(); -extern "C" const char *getUrlParam(); - -// BAD: QDesktopServices::openUrl with untrusted input -void bad1_qt(const char *userUrl) { - QUrl url(userUrl); - QDesktopServices::openUrl(url); // BAD -} - -void bad2_qt() { - const char *input = getUrlParam(); - QUrl url(input); - QDesktopServices::openUrl(url); // BAD -} - -void safe1_qt() { - QUrl url("https://example.com"); - QDesktopServices::openUrl(url); // GOOD - no taint -} - -void safe2_qt(const char *userUrl) { - if (strncmp(userUrl, "https://", 8) == 0 || - strncmp(userUrl, "http://", 7) == 0) { - QUrl url(userUrl); - QDesktopServices::openUrl(url); // GOOD - } -} - -void safe3_qt(QUrl &url) { - if (url.scheme() == "https" || url.scheme() == "http") { - QDesktopServices::openUrl(url); // GOOD - } -} \ No newline at end of file diff --git a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp index 1e63ba4..b5495da 100644 --- a/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp +++ b/java/src/security/PotentiallyUnguardedProtocolHandler/PotentiallyUnguardedProtocolHandler.qhelp @@ -14,6 +14,15 @@ they come from untrusted sources.

    + + +

    + In the bad example, an untrusted URL is passed directly to Desktop.browse() + without any scheme validation. + In the good example, the URL scheme is checked with getScheme() before + invoking the handler. +

    +
  • Positive Security: Allow