Skip to content

Commit 0afe22d

Browse files
smowtonsmowton
authored
Merge pull request github#5710 from p0wn4j/jsch-os-injection
[Java] CWE-078: Add JSch lib OS Command Injection sink
2 parents 498f9b2 + fd88b72 commit 0afe22d

File tree

14 files changed

+423
-0
lines changed

14 files changed

+423
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-078/ExecCommon.qll

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import semmle.code.java.dataflow.FlowSources
2+
import semmle.code.java.security.ExternalProcess
3+
import semmle.code.java.security.CommandArguments
4+
5+
private class RemoteUserInputToArgumentToExecFlowConfig extends TaintTracking::Configuration {
6+
RemoteUserInputToArgumentToExecFlowConfig() {
7+
this = "ExecCommon::RemoteUserInputToArgumentToExecFlowConfig"
8+
}
9+
10+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
11+
12+
override predicate isSink(DataFlow::Node sink) { sink.asExpr() instanceof ArgumentToExec }
13+
14+
override predicate isSanitizer(DataFlow::Node node) {
15+
node.getType() instanceof PrimitiveType
16+
or
17+
node.getType() instanceof BoxedType
18+
or
19+
isSafeCommandArgument(node.asExpr())
20+
}
21+
}
22+
23+
/**
24+
* Implementation of `ExecTainted.ql`. It is extracted to a QLL
25+
* so that it can be excluded from `ExecUnescaped.ql` to avoid
26+
* reporting overlapping results.
27+
*/
28+
predicate execTainted(DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg) {
29+
exists(RemoteUserInputToArgumentToExecFlowConfig conf |
30+
conf.hasFlowPath(source, sink) and sink.getNode() = DataFlow::exprNode(execArg)
31+
)
32+
}

‎java/ql/src/experimental/Security/CWE/CWE-078/ExecTainted.ql

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Uncontrolled command line
3+
* @description Using externally controlled strings in a command line is vulnerable to malicious
4+
* changes in the strings.
5+
* @kind path-problem
6+
* @problem.severity error
7+
* @precision high
8+
* @id java/command-line-injection
9+
* @tags security
10+
* external/cwe/cwe-078
11+
* external/cwe/cwe-088
12+
*/
13+
14+
import java
15+
import semmle.code.java.dataflow.FlowSources
16+
import semmle.code.java.security.ExternalProcess
17+
import ExecCommon
18+
import JSchOSInjection
19+
import DataFlow::PathGraph
20+
21+
from DataFlow::PathNode source, DataFlow::PathNode sink, ArgumentToExec execArg
22+
where execTainted(source, sink, execArg)
23+
select execArg, source, sink, "$@ flows to here and is used in a command.", source.getNode(),
24+
"User-provided value"

‎java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjection.qll

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* Provides classes for JSch OS command injection detection
3+
*/
4+
5+
import java
6+
7+
/** The class `com.jcraft.jsch.ChannelExec`. */
8+
private class JSchChannelExec extends RefType {
9+
JSchChannelExec() { this.hasQualifiedName("com.jcraft.jsch", "ChannelExec") }
10+
}
11+
12+
/** A method to set an OS Command for the execution. */
13+
private class ChannelExecSetCommandMethod extends Method, ExecCallable {
14+
ChannelExecSetCommandMethod() {
15+
this.hasName("setCommand") and
16+
this.getDeclaringType() instanceof JSchChannelExec
17+
}
18+
19+
override int getAnExecutedArgument() { result = 0 }
20+
}

‎java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjectionBad.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
public class JSchOSInjectionBad {
2+
void jschOsExecution(HttpServletRequest request) {
3+
String command = request.getParameter("command");
4+
5+
JSch jsch = new JSch();
6+
Session session = jsch.getSession("user", "sshHost", 22);
7+
session.setPassword("password");
8+
session.connect();
9+
10+
Channel channel = session.openChannel("exec");
11+
// BAD - untrusted user data is used directly in a command
12+
((ChannelExec) channel).setCommand("ping " + command);
13+
14+
channel.connect();
15+
}
16+
}
17+

‎java/ql/src/experimental/Security/CWE/CWE-078/JSchOSInjectionSanitized.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
public class JSchOSInjectionSanitized {
2+
void jschOsExecutionPing(HttpServletRequest request) {
3+
String untrusted = request.getParameter("command");
4+
5+
//GOOD - Validate user the input.
6+
if (!com.google.common.net.InetAddresses.isInetAddress(untrusted)) {
7+
System.out.println("Invalid IP address");
8+
return;
9+
}
10+
11+
JSch jsch = new JSch();
12+
Session session = jsch.getSession("user", "host", 22);
13+
session.setPassword("password");
14+
session.connect();
15+
16+
Channel channel = session.openChannel("exec");
17+
((ChannelExec) channel).setCommand("ping " + untrusted);
18+
19+
channel.connect();
20+
}
21+
22+
void jschOsExecutionDig(HttpServletRequest request) {
23+
String untrusted = request.getParameter("command");
24+
25+
//GOOD - check whether the user input doesn't contain dangerous shell characters.
26+
String[] badChars = new String[] {"^", "~" ," " , "&", "|", ";", "$", ">", "<", "`", "\\", ",", "!", "{", "}", "(", ")", "@", "%", "#", "%0A", "%0a", "\n", "\r\n"};
27+
28+
for (String badChar : badChars) {
29+
if (untrusted.contains(badChar)) {
30+
System.out.println("Invalid host");
31+
return;
32+
}
33+
}
34+
35+
JSch jsch = new JSch();
36+
Session session = jsch.getSession("user", "host", 22);
37+
session.setPassword("password");
38+
session.connect();
39+
40+
Channel channel = session.openChannel("exec");
41+
((ChannelExec) channel).setCommand("dig " + untrusted);
42+
43+
channel.connect();
44+
}
45+
}
46+

‎java/ql/test/experimental/query-tests/security/CWE-078/ExecTainted.expected

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
edges
2+
| JSchOSInjectionTest.java:14:30:14:60 | getParameter(...) : String | JSchOSInjectionTest.java:26:48:26:64 | ... + ... |
3+
| JSchOSInjectionTest.java:38:30:38:60 | getParameter(...) : String | JSchOSInjectionTest.java:50:32:50:48 | ... + ... |
4+
nodes
5+
| JSchOSInjectionTest.java:14:30:14:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
6+
| JSchOSInjectionTest.java:26:48:26:64 | ... + ... | semmle.label | ... + ... |
7+
| JSchOSInjectionTest.java:38:30:38:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
8+
| JSchOSInjectionTest.java:50:32:50:48 | ... + ... | semmle.label | ... + ... |
9+
#select
10+
| JSchOSInjectionTest.java:26:48:26:64 | ... + ... | JSchOSInjectionTest.java:14:30:14:60 | getParameter(...) : String | JSchOSInjectionTest.java:26:48:26:64 | ... + ... | $@ flows to here and is used in a command. | JSchOSInjectionTest.java:14:30:14:60 | getParameter(...) | User-provided value |
11+
| JSchOSInjectionTest.java:50:32:50:48 | ... + ... | JSchOSInjectionTest.java:38:30:38:60 | getParameter(...) : String | JSchOSInjectionTest.java:50:32:50:48 | ... + ... | $@ flows to here and is used in a command. | JSchOSInjectionTest.java:38:30:38:60 | getParameter(...) | User-provided value |

‎java/ql/test/experimental/query-tests/security/CWE-078/ExecTainted.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-078/ExecTainted.ql

‎java/ql/test/experimental/query-tests/security/CWE-078/JSchOSInjectionTest.java

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import com.jcraft.jsch.*;
2+
3+
import javax.servlet.http.*;
4+
import javax.servlet.ServletException;
5+
import java.io.IOException;
6+
7+
public class JSchOSInjectionTest extends HttpServlet {
8+
9+
protected void doGet(HttpServletRequest request, HttpServletResponse response)
10+
throws ServletException, IOException {
11+
String host = "sshHost";
12+
String user = "user";
13+
String password = "password";
14+
String command = request.getParameter("command");
15+
16+
java.util.Properties config = new java.util.Properties();
17+
config.put("StrictHostKeyChecking", "no");
18+
19+
JSch jsch = new JSch();
20+
Session session = jsch.getSession(user, host, 22);
21+
session.setPassword(password);
22+
session.setConfig(config);
23+
session.connect();
24+
25+
Channel channel = session.openChannel("exec");
26+
((ChannelExec) channel).setCommand("ping " + command);
27+
channel.setInputStream(null);
28+
((ChannelExec) channel).setErrStream(System.err);
29+
30+
channel.connect();
31+
}
32+
33+
protected void doPost(HttpServletRequest request, HttpServletResponse response)
34+
throws ServletException, IOException {
35+
String host = "sshHost";
36+
String user = "user";
37+
String password = "password";
38+
String command = request.getParameter("command");
39+
40+
java.util.Properties config = new java.util.Properties();
41+
config.put("StrictHostKeyChecking", "no");
42+
43+
JSch jsch = new JSch();
44+
Session session = jsch.getSession(user, host, 22);
45+
session.setPassword(password);
46+
session.setConfig(config);
47+
session.connect();
48+
49+
ChannelExec channel = (ChannelExec)session.openChannel("exec");
50+
channel.setCommand("ping " + command);
51+
channel.setInputStream(null);
52+
channel.setErrStream(System.err);
53+
54+
channel.connect();
55+
}
56+
}

‎java/ql/test/experimental/query-tests/security/CWE-078/options

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4:${testdir}/../../../stubs/jsch-0.1.55
2+

‎java/ql/test/experimental/stubs/jsch-0.1.55/com/jcraft/jsch/Channel.java

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
/* -*-mode:java; c-basic-offset:2; indent-tabs-mode:nil -*- */
2+
/*
3+
Copyright (c) 2002-2018 ymnk, JCraft,Inc. All rights reserved.
4+
5+
Redistribution and use in source and binary forms, with or without
6+
modification, are permitted provided that the following conditions are met:
7+
8+
1. Redistributions of source code must retain the above copyright notice,
9+
this list of conditions and the following disclaimer.
10+
11+
2. Redistributions in binary form must reproduce the above copyright
12+
notice, this list of conditions and the following disclaimer in
13+
the documentation and/or other materials provided with the distribution.
14+
15+
3. The names of the authors may not be used to endorse or promote products
16+
derived from this software without specific prior written permission.
17+
18+
THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESSED OR IMPLIED WARRANTIES,
19+
INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
20+
FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL JCRAFT,
21+
INC. OR ANY CONTRIBUTORS TO THIS SOFTWARE BE LIABLE FOR ANY DIRECT, INDIRECT,
22+
INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
23+
LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA,
24+
OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
25+
LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
26+
NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
27+
EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
28+
*/
29+
30+
package com.jcraft.jsch;
31+
32+
import java.io.*;
33+
34+
public abstract class Channel implements Runnable {
35+
public void connect() {
36+
}
37+
38+
public void setInputStream(InputStream in){
39+
}
40+
41+
public void disconnect(){
42+
}
43+
44+
public void run() {
45+
}
46+
}

0 commit comments

Comments
 (0)