Skip to content

Commit 3400c12

Browse files
authored
Merge pull request github#5202 from joefarebrother/apache-http
Java: Add modelling for Apache HTTP Components
2 parents 73ad417 + 81ff768 commit 3400c12

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+3383
-30
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
lgtm,codescanning
2+
* Added support for the Apache Http components core library (`org.apache.http.*` and `org.apache.hc.core5.*`); adding additional remote flow sources, sinks for the XSS and Open Redirect queries, and additional taint steps.

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ import java
6464
private import semmle.code.java.dataflow.DataFlow::DataFlow
6565
private import internal.DataFlowPrivate
6666

67+
/**
68+
* A module importing the frameworks that provide external flow data,
69+
* ensuring that they are visible to the taint tracking / data flow library.
70+
*/
71+
private module Frameworks {
72+
private import semmle.code.java.frameworks.ApacheHttp
73+
}
74+
6775
private predicate sourceModelCsv(string row) {
6876
row =
6977
[
@@ -214,7 +222,7 @@ module CsvValidation {
214222
not name.regexpMatch("[a-zA-Z0-9_]*") and
215223
msg = "Dubious name \"" + name + "\" in " + pred + " model."
216224
or
217-
not signature.regexpMatch("|\\([a-zA-Z0-9_\\.\\$<>,]*\\)") and
225+
not signature.regexpMatch("|\\([a-zA-Z0-9_\\.\\$<>,\\[\\]]*\\)") and
218226
msg = "Dubious signature \"" + signature + "\" in " + pred + " model."
219227
or
220228
not ext.regexpMatch("|Annotated") and

java/ql/src/semmle/code/java/dataflow/FlowSteps.qll

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,15 @@ private import semmle.code.java.dataflow.DataFlow
99
* A module importing the frameworks that implement additional flow steps,
1010
* ensuring that they are visible to the taint tracking library.
1111
*/
12-
module Frameworks {
12+
private module Frameworks {
1313
private import semmle.code.java.frameworks.jackson.JacksonSerializability
1414
private import semmle.code.java.frameworks.android.Intent
1515
private import semmle.code.java.frameworks.android.SQLite
1616
private import semmle.code.java.frameworks.Guice
1717
private import semmle.code.java.frameworks.Protobuf
1818
private import semmle.code.java.frameworks.guava.Guava
1919
private import semmle.code.java.frameworks.apache.Lang
20+
private import semmle.code.java.frameworks.ApacheHttp
2021
}
2122

2223
/**

java/ql/src/semmle/code/java/frameworks/ApacheHttp.qll

Lines changed: 192 additions & 1 deletion
Large diffs are not rendered by default.

java/ql/src/semmle/code/java/security/UrlRedirect.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import java
44
import semmle.code.java.dataflow.DataFlow
55
import semmle.code.java.frameworks.Servlets
6+
import semmle.code.java.frameworks.ApacheHttp
67

78
/** A URL redirection sink */
89
abstract class UrlRedirectSink extends DataFlow::Node { }
@@ -24,3 +25,13 @@ private class ServletUrlRedirectSink extends UrlRedirectSink {
2425
)
2526
}
2627
}
28+
29+
/** A URL redirection sink from Apache Http components. */
30+
private class ApacheUrlRedirectSink extends UrlRedirectSink {
31+
ApacheUrlRedirectSink() {
32+
exists(ApacheHttpSetHeader c |
33+
c.getName().(CompileTimeConstantExpr).getStringValue() = "Location" and
34+
this.asExpr() = c.getValue()
35+
)
36+
}
37+
}

java/ql/src/semmle/code/java/security/XSS.qll

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import semmle.code.java.frameworks.spring.SpringController
77
import semmle.code.java.frameworks.spring.SpringHttp
88
import semmle.code.java.dataflow.DataFlow
99
import semmle.code.java.dataflow.TaintTracking2
10+
import semmle.code.java.dataflow.ExternalFlow
1011

1112
/** A sink that represent a method that outputs data without applying contextual output encoding. */
1213
abstract class XssSink extends DataFlow::Node { }
@@ -31,6 +32,8 @@ class XssAdditionalTaintStep extends Unit {
3132
/** A default sink representing methods susceptible to XSS attacks. */
3233
private class DefaultXssSink extends XssSink {
3334
DefaultXssSink() {
35+
sinkNode(this, "xss")
36+
or
3437
exists(HttpServletResponseSendErrorMethod m, MethodAccess ma |
3538
ma.getMethod() = m and
3639
this.asExpr() = ma.getArgument(1)
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import org.apache.http.*;
2+
import org.apache.http.protocol.*;
3+
import org.apache.http.message.BasicHeader;
4+
import org.apache.http.util.*;
5+
import org.apache.http.entity.*;
6+
import java.io.IOException;
7+
8+
class A {
9+
static Object taint() { return null; }
10+
11+
static void sink(Object o) { }
12+
13+
class Test1 implements HttpRequestHandler {
14+
public void handle(HttpRequest req, HttpResponse res, HttpContext ctx) throws IOException {
15+
A.sink(req.getRequestLine()); //$hasTaintFlow=y
16+
A.sink(req.getRequestLine().getUri()); //$hasTaintFlow=y
17+
A.sink(req.getRequestLine().getMethod()); //$hasTaintFlow=y
18+
A.sink(req.getAllHeaders()); //$hasTaintFlow=y
19+
HeaderIterator it = req.headerIterator();
20+
A.sink(it.next()); //$hasTaintFlow=y
21+
A.sink(it.nextHeader()); //$hasTaintFlow=y
22+
Header h = req.getHeaders("abc")[3];
23+
A.sink(h.getName()); //$hasTaintFlow=y
24+
A.sink(h.getValue()); //$hasTaintFlow=y
25+
HeaderElement el = h.getElements()[0];
26+
A.sink(el.getName()); //$hasTaintFlow=y
27+
A.sink(el.getValue()); //$hasTaintFlow=y
28+
A.sink(el.getParameters()); //$hasTaintFlow=y
29+
A.sink(el.getParameterByName("abc").getValue()); //$hasTaintFlow=y
30+
A.sink(el.getParameter(0).getName()); //$hasTaintFlow=y
31+
HttpEntity ent = ((HttpEntityEnclosingRequest)req).getEntity();
32+
A.sink(ent.getContent()); //$hasTaintFlow=y
33+
A.sink(ent.getContentEncoding()); //$hasTaintFlow=y
34+
A.sink(ent.getContentType()); //$hasTaintFlow=y
35+
A.sink(EntityUtils.toString(ent)); //$hasTaintFlow=y
36+
A.sink(EntityUtils.toByteArray(ent)); //$hasTaintFlow=y
37+
A.sink(EntityUtils.getContentCharSet(ent)); //$hasTaintFlow=y
38+
A.sink(EntityUtils.getContentMimeType(ent)); //$hasTaintFlow=y
39+
res.setEntity(new StringEntity("<a href='" + req.getRequestLine().getUri() + "'>a</a>")); //$hasTaintFlow=y
40+
EntityUtils.updateEntity(res, new ByteArrayEntity(EntityUtils.toByteArray(ent))); //$hasTaintFlow=y
41+
res.setHeader("Location", req.getRequestLine().getUri()); //$hasTaintFlow=y
42+
res.setHeader(new BasicHeader("Location", req.getRequestLine().getUri())); //$hasTaintFlow=y
43+
}
44+
}
45+
46+
void test2() {
47+
ByteArrayBuffer bbuf = new ByteArrayBuffer(42);
48+
bbuf.append((byte[]) taint(), 0, 3);
49+
sink(bbuf.buffer()); //$hasTaintFlow=y
50+
sink(bbuf.toByteArray()); //$hasTaintFlow=y
51+
52+
CharArrayBuffer cbuf = new CharArrayBuffer(42);
53+
cbuf.append(bbuf.toByteArray(), 0, 3);
54+
sink(cbuf.toCharArray()); //$hasTaintFlow=y
55+
sink(cbuf.toString()); //$hasTaintFlow=y
56+
sink(cbuf.subSequence(0, 3)); //$hasTaintFlow=y
57+
sink(cbuf.substring(0, 3)); //$hasTaintFlow=y
58+
sink(cbuf.substringTrimmed(0, 3)); //$hasTaintFlow=y
59+
60+
sink(Args.notNull(taint(), "x")); //$hasTaintFlow=y
61+
sink(Args.notEmpty((String) taint(), "x")); //$hasTaintFlow=y
62+
sink(Args.notBlank((String) taint(), "x")); //$hasTaintFlow=y
63+
sink(Args.notNull("x", (String) taint())); // Good
64+
}
65+
}
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import org.apache.hc.core5.http.*;
2+
import org.apache.hc.core5.http.protocol.HttpContext;
3+
import org.apache.hc.core5.http.io.HttpRequestHandler;
4+
import org.apache.hc.core5.http.io.HttpServerRequestHandler;
5+
import org.apache.hc.core5.http.message.*;
6+
import org.apache.hc.core5.http.io.entity.*;
7+
import org.apache.hc.core5.util.*;
8+
import java.io.IOException;
9+
10+
class B {
11+
static Object taint() { return null; }
12+
13+
static void sink(Object o) { }
14+
15+
class Test1 implements HttpRequestHandler {
16+
public void handle(ClassicHttpRequest req, ClassicHttpResponse res, HttpContext ctx) throws IOException, ParseException {
17+
B.sink(req.getAuthority().getHostName()); //$hasTaintFlow=y
18+
B.sink(req.getAuthority().toString()); //$hasTaintFlow=y
19+
B.sink(req.getMethod()); //$hasTaintFlow=y
20+
B.sink(req.getPath()); //$hasTaintFlow=y
21+
B.sink(req.getScheme());
22+
B.sink(req.getRequestUri()); //$hasTaintFlow=y
23+
RequestLine line = new RequestLine(req);
24+
B.sink(line.getUri()); //$hasTaintFlow=y
25+
B.sink(line.getMethod()); //$hasTaintFlow=y
26+
B.sink(req.getHeaders()); //$hasTaintFlow=y
27+
B.sink(req.headerIterator()); //$hasTaintFlow=y
28+
Header h = req.getHeaders("abc")[3];
29+
B.sink(h.getName()); //$hasTaintFlow=y
30+
B.sink(h.getValue()); //$hasTaintFlow=y
31+
B.sink(req.getFirstHeader("abc")); //$hasTaintFlow=y
32+
B.sink(req.getLastHeader("abc")); //$hasTaintFlow=y
33+
HttpEntity ent = req.getEntity();
34+
B.sink(ent.getContent()); //$hasTaintFlow=y
35+
B.sink(ent.getContentEncoding()); //$hasTaintFlow=y
36+
B.sink(ent.getContentType()); //$hasTaintFlow=y
37+
B.sink(ent.getTrailerNames()); //$hasTaintFlow=y
38+
B.sink(ent.getTrailers().get()); //$hasTaintFlow=y
39+
B.sink(EntityUtils.toString(ent)); //$hasTaintFlow=y
40+
B.sink(EntityUtils.toByteArray(ent)); //$hasTaintFlow=y
41+
B.sink(EntityUtils.parse(ent)); //$hasTaintFlow=y
42+
res.setEntity(new StringEntity("<a href='" + req.getRequestUri() + "'>a</a>")); //$hasTaintFlow=y
43+
res.setEntity(new ByteArrayEntity(EntityUtils.toByteArray(ent), ContentType.TEXT_HTML)); //$hasTaintFlow=y
44+
res.setEntity(HttpEntities.create("<a href='" + req.getRequestUri() + "'>a</a>")); //$hasTaintFlow=y
45+
res.setHeader("Location", req.getRequestUri()); //$hasTaintFlow=y
46+
res.setHeader(new BasicHeader("Location", req.getRequestUri())); //$hasTaintFlow=y
47+
}
48+
}
49+
50+
void test2() {
51+
ByteArrayBuffer bbuf = new ByteArrayBuffer(42);
52+
bbuf.append((byte[]) taint(), 0, 3);
53+
sink(bbuf.array()); //$hasTaintFlow=y
54+
sink(bbuf.toByteArray()); //$hasTaintFlow=y
55+
sink(bbuf.toString());
56+
57+
CharArrayBuffer cbuf = new CharArrayBuffer(42);
58+
cbuf.append(bbuf.toByteArray(), 0, 3);
59+
sink(cbuf.toCharArray()); //$hasTaintFlow=y
60+
sink(cbuf.toString()); //$hasTaintFlow=y
61+
sink(cbuf.subSequence(0, 3)); //$hasTaintFlow=y
62+
sink(cbuf.substring(0, 3)); //$hasTaintFlow=y
63+
sink(cbuf.substringTrimmed(0, 3)); //$hasTaintFlow=y
64+
65+
sink(Args.notNull(taint(), "x")); //$hasTaintFlow=y
66+
sink(Args.notEmpty((String) taint(), "x")); //$hasTaintFlow=y
67+
sink(Args.notBlank((String) taint(), "x")); //$hasTaintFlow=y
68+
sink(Args.notNull("x", (String) taint()));
69+
}
70+
71+
class Test3 implements HttpServerRequestHandler {
72+
public void handle(ClassicHttpRequest req, HttpServerRequestHandler.ResponseTrigger restr, HttpContext ctx) throws HttpException, IOException {
73+
B.sink(req.getEntity()); //$hasTaintFlow=y
74+
}
75+
}
76+
}

java/ql/test/library-tests/frameworks/apache-http/flow.expected

Whitespace-only changes.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import java
2+
import semmle.code.java.dataflow.TaintTracking
3+
import semmle.code.java.dataflow.FlowSources
4+
import semmle.code.java.security.XSS
5+
import semmle.code.java.security.UrlRedirect
6+
import TestUtilities.InlineExpectationsTest
7+
8+
class Conf extends TaintTracking::Configuration {
9+
Conf() { this = "qltest:frameworks:apache-http" }
10+
11+
override predicate isSource(DataFlow::Node n) {
12+
n.asExpr().(MethodAccess).getMethod().hasName("taint")
13+
or
14+
n instanceof RemoteFlowSource
15+
}
16+
17+
override predicate isSink(DataFlow::Node n) {
18+
exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
19+
or
20+
n instanceof XssSink
21+
or
22+
n instanceof UrlRedirectSink
23+
}
24+
}
25+
26+
class HasFlowTest extends InlineExpectationsTest {
27+
HasFlowTest() { this = "HasFlowTest" }
28+
29+
override string getARelevantTag() { result = "hasTaintFlow" }
30+
31+
override predicate hasActualResult(Location location, string element, string tag, string value) {
32+
tag = "hasTaintFlow" and
33+
exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) |
34+
sink.getLocation() = location and
35+
element = sink.toString() and
36+
value = "y"
37+
)
38+
}
39+
}

0 commit comments

Comments
 (0)