Skip to content

Commit 5874ecc

Browse files
aibaarsaibaars
authored
Merge pull request github#3976 from luchua-bc/java-unsecure-basic-auth
Java: Insecure basic authentication
2 parents 1b0cfc9 + b821f91 commit 5874ecc

29 files changed

+2715
-0
lines changed

‎java/ql/src/experimental/Security/CWE/CWE-522/InsecureBasicAuth.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
public class InsecureBasicAuth {
2+
/**
3+
* Test basic authentication with Apache HTTP request.
4+
*/
5+
public void testApacheHttpRequest(String username, String password) {
6+
{
7+
// BAD: basic authentication over HTTP
8+
String url = "http://www.example.com/rest/getuser.do?uid=abcdx";
9+
}
10+
11+
{
12+
// GOOD: basic authentication over HTTPS
13+
String url = "https://www.example.com/rest/getuser.do?uid=abcdx";
14+
}
15+
16+
HttpPost post = new HttpPost(url);
17+
post.setHeader("Accept", "application/json");
18+
post.setHeader("Content-type", "application/json");
19+
20+
String authString = username + ":" + password;
21+
byte[] authEncBytes = Base64.getEncoder().encode(authString.getBytes());
22+
String authStringEnc = new String(authEncBytes);
23+
24+
post.addHeader("Authorization", "Basic " + authStringEnc);
25+
}
26+
27+
/**
28+
* Test basic authentication with Java HTTP URL connection.
29+
*/
30+
public void testHttpUrlConnection(String username, String password) {
31+
{
32+
// BAD: basic authentication over HTTP
33+
String urlStr = "http://www.example.com/rest/getuser.do?uid=abcdx";
34+
}
35+
36+
{
37+
// GOOD: basic authentication over HTTPS
38+
String urlStr = "https://www.example.com/rest/getuser.do?uid=abcdx";
39+
}
40+
41+
String authString = username + ":" + password;
42+
String encoding = Base64.getEncoder().encodeToString(authString.getBytes("UTF-8"));
43+
URL url = new URL(urlStr);
44+
HttpURLConnection conn = (HttpURLConnection) url.openConnection();
45+
conn.setRequestMethod("POST");
46+
conn.setDoOutput(true);
47+
conn.setRequestProperty("Authorization", "Basic " + encoding);
48+
}
49+
}

‎java/ql/src/experimental/Security/CWE/CWE-522/InsecureBasicAuth.qhelp

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>Basic authentication only obfuscates username/password in Base64 encoding, which can be easily recognized and reversed, thus it must not be transmitted over the cleartext HTTP channel. Transmission of sensitive information not in HTTPS is vulnerable to packet sniffing.</p>
6+
</overview>
7+
8+
<recommendation>
9+
<p>Either use a more secure authentication mechanism like digest authentication or federated authentication, or use the HTTPS communication protocol.</p>
10+
</recommendation>
11+
12+
<example>
13+
<p>The following example shows two ways of using basic authentication. In the 'BAD' case, the credentials are transmitted over HTTP. In the 'GOOD' case, the credentials are transmitted over HTTPS.</p>
14+
<sample src="InsecureBasicAuth.java" />
15+
</example>
16+
17+
<references>
18+
<li>
19+
<a href="https://cwe.mitre.org/data/definitions/522">CWE-522</a>
20+
</li>
21+
<li>
22+
SonarSource rule:
23+
<a href="https://rules.sonarsource.com/java/tag/owasp/RSPEC-2647">Basic authentication should not be used</a>
24+
</li>
25+
<li>
26+
Acunetix:
27+
<a href="https://www.acunetix.com/vulnerabilities/web/basic-authentication-over-http/">WEB VULNERABILITIES INDEX - Basic authentication over HTTP</a>
28+
</li>
29+
</references>
30+
</qhelp>

‎java/ql/src/experimental/Security/CWE/CWE-522/InsecureBasicAuth.ql

Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
/**
2+
* @name Insecure basic authentication
3+
* @description Basic authentication only obfuscates username/password in Base64 encoding, which can be easily recognized and reversed. Transmission of sensitive information not over HTTPS is vulnerable to packet sniffing.
4+
* @kind path-problem
5+
* @id java/insecure-basic-auth
6+
* @tags security
7+
* external/cwe-522
8+
* external/cwe-319
9+
*/
10+
11+
import java
12+
import semmle.code.java.frameworks.Networking
13+
import semmle.code.java.dataflow.TaintTracking
14+
import DataFlow::PathGraph
15+
16+
/**
17+
* Gets a regular expression for matching private hosts, which only matches the host portion therefore checking for port is not necessary.
18+
*/
19+
private string getPrivateHostRegex() {
20+
result =
21+
"(?i)localhost(?:[:/?#].*)?|127\\.0\\.0\\.1(?:[:/?#].*)?|10(?:\\.[0-9]+){3}(?:[:/?#].*)?|172\\.16(?:\\.[0-9]+){2}(?:[:/?#].*)?|192.168(?:\\.[0-9]+){2}(?:[:/?#].*)?|\\[?0:0:0:0:0:0:0:1\\]?(?:[:/?#].*)?|\\[?::1\\]?(?:[:/?#].*)?"
22+
}
23+
24+
/**
25+
* The Java class `org.apache.http.client.methods.HttpRequestBase`. Popular subclasses include `HttpGet`, `HttpPost`, and `HttpPut`.
26+
* And the Java class `org.apache.http.message.BasicHttpRequest`.
27+
*/
28+
class ApacheHttpRequest extends RefType {
29+
ApacheHttpRequest() {
30+
this
31+
.getASourceSupertype*()
32+
.hasQualifiedName("org.apache.http.client.methods", "HttpRequestBase") or
33+
this.getASourceSupertype*().hasQualifiedName("org.apache.http.message", "BasicHttpRequest")
34+
}
35+
}
36+
37+
/**
38+
* Class of Java URL constructor.
39+
*/
40+
class URLConstructor extends ClassInstanceExpr {
41+
URLConstructor() { this.getConstructor().getDeclaringType() instanceof TypeUrl }
42+
43+
predicate hasHttpStringArg() {
44+
this.getConstructor().getParameter(0).getType() instanceof TypeString and
45+
(
46+
// URLs constructed with any of the three string constructors below:
47+
// `URL(String protocol, String host, int port, String file)`,
48+
// `URL(String protocol, String host, int port, String file, URLStreamHandler handler)`,
49+
// `URL(String protocol, String host, String file)`
50+
this.getConstructor().getNumberOfParameters() > 1 and
51+
concatHttpString(getArgument(0), this.getArgument(1)) // First argument contains the protocol part and the second argument contains the host part.
52+
or
53+
// URLs constructed with the string constructor `URL(String spec)`
54+
this.getConstructor().getNumberOfParameters() = 1 and
55+
this.getArgument(0) instanceof HttpString // First argument contains the whole spec.
56+
)
57+
}
58+
}
59+
60+
/**
61+
* Class of Java URI constructor.
62+
*/
63+
class URIConstructor extends ClassInstanceExpr {
64+
URIConstructor() { this.getConstructor().getDeclaringType().hasQualifiedName("java.net", "URI") }
65+
66+
predicate hasHttpStringArg() {
67+
(
68+
this.getNumArgument() = 1 and
69+
this.getArgument(0) instanceof HttpString // `URI(String str)`
70+
or
71+
this.getNumArgument() = 4 and
72+
concatHttpString(this.getArgument(0), this.getArgument(1)) // `URI(String scheme, String host, String path, String fragment)`
73+
or
74+
this.getNumArgument() = 5 and
75+
concatHttpString(this.getArgument(0), this.getArgument(1)) // `URI(String scheme, String authority, String path, String query, String fragment)` without user-info in authority
76+
or
77+
this.getNumArgument() = 7 and
78+
concatHttpString(this.getArgument(0), this.getArgument(2)) // `URI(String scheme, String userInfo, String host, int port, String path, String query, String fragment)`
79+
)
80+
}
81+
}
82+
83+
/**
84+
* String of HTTP URLs not in private domains.
85+
*/
86+
class HttpStringLiteral extends StringLiteral {
87+
HttpStringLiteral() {
88+
// Match URLs with the HTTP protocol and without private IP addresses to reduce false positives.
89+
exists(string s | this.getRepresentedString() = s |
90+
s.regexpMatch("(?i)http://[\\[a-zA-Z0-9].*") and
91+
not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex())
92+
)
93+
}
94+
}
95+
96+
/**
97+
* Checks both parts of protocol and host.
98+
*/
99+
predicate concatHttpString(Expr protocol, Expr host) {
100+
(
101+
protocol.(CompileTimeConstantExpr).getStringValue().regexpMatch("(?i)http(://)?") or
102+
protocol
103+
.(VarAccess)
104+
.getVariable()
105+
.getAnAssignedValue()
106+
.(CompileTimeConstantExpr)
107+
.getStringValue()
108+
.regexpMatch("(?i)http(://)?")
109+
) and
110+
not exists(string hostString |
111+
hostString = host.(CompileTimeConstantExpr).getStringValue() or
112+
hostString =
113+
host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue()
114+
|
115+
hostString.length() = 0 or // Empty host is loopback address
116+
hostString.regexpMatch(getPrivateHostRegex())
117+
)
118+
}
119+
120+
/** Gets the leftmost operand in a concatenated string */
121+
Expr getLeftmostConcatOperand(Expr expr) {
122+
if expr instanceof AddExpr
123+
then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand())
124+
else result = expr
125+
}
126+
127+
/**
128+
* String concatenated with `HttpStringLiteral`.
129+
*/
130+
class HttpString extends Expr {
131+
HttpString() {
132+
this instanceof HttpStringLiteral
133+
or
134+
concatHttpString(this.(AddExpr).getLeftOperand(),
135+
getLeftmostConcatOperand(this.(AddExpr).getRightOperand()))
136+
}
137+
}
138+
139+
/**
140+
* String pattern of basic authentication.
141+
*/
142+
class BasicAuthString extends StringLiteral {
143+
BasicAuthString() { exists(string s | this.getRepresentedString() = s | s.matches("Basic %")) }
144+
}
145+
146+
/**
147+
* String concatenated with `BasicAuthString`.
148+
*/
149+
predicate builtFromBasicAuthStringConcat(Expr expr) {
150+
expr instanceof BasicAuthString
151+
or
152+
builtFromBasicAuthStringConcat(expr.(AddExpr).getLeftOperand())
153+
or
154+
exists(Expr other | builtFromBasicAuthStringConcat(other) |
155+
exists(Variable var | var.getAnAssignedValue() = other and var.getAnAccess() = expr)
156+
)
157+
}
158+
159+
/** The `openConnection` method of Java URL. Not to include `openStream` since it won't be used in this query. */
160+
class HttpURLOpenMethod extends Method {
161+
HttpURLOpenMethod() {
162+
this.getDeclaringType() instanceof TypeUrl and
163+
this.getName() = "openConnection"
164+
}
165+
}
166+
167+
/** Constructor of `ApacheHttpRequest` */
168+
predicate apacheHttpRequest(DataFlow::Node node1, DataFlow::Node node2) {
169+
exists(ConstructorCall cc |
170+
cc.getConstructedType() instanceof ApacheHttpRequest and
171+
node2.asExpr() = cc and
172+
cc.getAnArgument() = node1.asExpr()
173+
)
174+
}
175+
176+
/** `URI` methods */
177+
predicate createURI(DataFlow::Node node1, DataFlow::Node node2) {
178+
exists(
179+
URIConstructor cc // new URI
180+
|
181+
node2.asExpr() = cc and
182+
cc.getArgument(0) = node1.asExpr()
183+
)
184+
or
185+
exists(
186+
StaticMethodAccess ma // URI.create
187+
|
188+
ma.getMethod().getDeclaringType().hasQualifiedName("java.net", "URI") and
189+
ma.getMethod().hasName("create") and
190+
node1.asExpr() = ma.getArgument(0) and
191+
node2.asExpr() = ma
192+
)
193+
}
194+
195+
/** Constructors of `URL` */
196+
predicate createURL(DataFlow::Node node1, DataFlow::Node node2) {
197+
exists(URLConstructor cc |
198+
node2.asExpr() = cc and
199+
cc.getArgument(0) = node1.asExpr()
200+
)
201+
}
202+
203+
/** Method call of `HttpURLOpenMethod` */
204+
predicate urlOpen(DataFlow::Node node1, DataFlow::Node node2) {
205+
exists(MethodAccess ma |
206+
ma.getMethod() instanceof HttpURLOpenMethod and
207+
node1.asExpr() = ma.getQualifier() and
208+
ma = node2.asExpr()
209+
)
210+
}
211+
212+
/** Constructor of `BasicRequestLine` */
213+
predicate basicRequestLine(DataFlow::Node node1, DataFlow::Node node2) {
214+
exists(ConstructorCall mcc |
215+
mcc.getConstructedType().hasQualifiedName("org.apache.http.message", "BasicRequestLine") and
216+
mcc.getArgument(1) = node1.asExpr() and // `BasicRequestLine(String method, String uri, ProtocolVersion version)
217+
node2.asExpr() = mcc
218+
)
219+
}
220+
221+
class BasicAuthFlowConfig extends TaintTracking::Configuration {
222+
BasicAuthFlowConfig() { this = "InsecureBasicAuth::BasicAuthFlowConfig" }
223+
224+
override predicate isSource(DataFlow::Node src) {
225+
src.asExpr() instanceof HttpString
226+
or
227+
exists(URLConstructor uc |
228+
uc.hasHttpStringArg() and
229+
src.asExpr() = uc.getArgument(0)
230+
)
231+
or
232+
exists(URIConstructor uc |
233+
uc.hasHttpStringArg() and
234+
src.asExpr() = uc.getArgument(0)
235+
)
236+
}
237+
238+
override predicate isSink(DataFlow::Node sink) {
239+
exists(MethodAccess ma |
240+
sink.asExpr() = ma.getQualifier() and
241+
(
242+
ma.getMethod().hasName("addHeader") or
243+
ma.getMethod().hasName("setHeader") or
244+
ma.getMethod().hasName("setRequestProperty")
245+
) and
246+
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "Authorization" and
247+
builtFromBasicAuthStringConcat(ma.getArgument(1))
248+
)
249+
}
250+
251+
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
252+
apacheHttpRequest(node1, node2) or
253+
createURI(node1, node2) or
254+
basicRequestLine(node1, node2) or
255+
createURL(node1, node2) or
256+
urlOpen(node1, node2)
257+
}
258+
}
259+
260+
from DataFlow::PathNode source, DataFlow::PathNode sink, BasicAuthFlowConfig config
261+
where config.hasFlowPath(source, sink)
262+
select sink.getNode(), source, sink, "Insecure basic authentication from $@.", source.getNode(),
263+
"HTTP url"

‎java/ql/test/experimental/query-tests/security/CWE-522/InsecureBasicAuth.expected

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
edges
2+
| InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | InsecureBasicAuth.java:28:3:28:6 | post |
3+
| InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get |
4+
| InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:54:3:54:6 | post |
5+
| InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:71:3:71:6 | post |
6+
| InsecureBasicAuth.java:78:47:78:52 | "http" : String | InsecureBasicAuth.java:86:3:86:6 | post |
7+
| InsecureBasicAuth.java:93:19:93:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:102:3:102:6 | post |
8+
| InsecureBasicAuth.java:109:19:109:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:119:3:119:6 | post |
9+
| InsecureBasicAuth.java:126:19:126:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:130:28:130:67 | (...)... : URLConnection |
10+
| InsecureBasicAuth.java:130:28:130:67 | (...)... : URLConnection | InsecureBasicAuth.java:133:3:133:6 | conn |
11+
| InsecureBasicAuth.java:145:21:145:28 | protocol : String | InsecureBasicAuth.java:146:28:146:67 | (...)... : URLConnection |
12+
| InsecureBasicAuth.java:146:28:146:67 | (...)... : URLConnection | InsecureBasicAuth.java:149:3:149:6 | conn |
13+
nodes
14+
| InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | semmle.label | ... + ... : String |
15+
| InsecureBasicAuth.java:28:3:28:6 | post | semmle.label | post |
16+
| InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | semmle.label | "http://www.example.com:8000/payment/retrieve" : String |
17+
| InsecureBasicAuth.java:38:3:38:5 | get | semmle.label | get |
18+
| InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
19+
| InsecureBasicAuth.java:54:3:54:6 | post | semmle.label | post |
20+
| InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
21+
| InsecureBasicAuth.java:71:3:71:6 | post | semmle.label | post |
22+
| InsecureBasicAuth.java:78:47:78:52 | "http" : String | semmle.label | "http" : String |
23+
| InsecureBasicAuth.java:86:3:86:6 | post | semmle.label | post |
24+
| InsecureBasicAuth.java:93:19:93:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
25+
| InsecureBasicAuth.java:102:3:102:6 | post | semmle.label | post |
26+
| InsecureBasicAuth.java:109:19:109:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
27+
| InsecureBasicAuth.java:119:3:119:6 | post | semmle.label | post |
28+
| InsecureBasicAuth.java:126:19:126:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | semmle.label | "http://www.example.com/rest/getuser.do?uid=abcdx" : String |
29+
| InsecureBasicAuth.java:130:28:130:67 | (...)... : URLConnection | semmle.label | (...)... : URLConnection |
30+
| InsecureBasicAuth.java:133:3:133:6 | conn | semmle.label | conn |
31+
| InsecureBasicAuth.java:145:21:145:28 | protocol : String | semmle.label | protocol : String |
32+
| InsecureBasicAuth.java:146:28:146:67 | (...)... : URLConnection | semmle.label | (...)... : URLConnection |
33+
| InsecureBasicAuth.java:149:3:149:6 | conn | semmle.label | conn |
34+
#select
35+
| InsecureBasicAuth.java:28:3:28:6 | post | InsecureBasicAuth.java:20:39:20:52 | ... + ... : String | InsecureBasicAuth.java:28:3:28:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:20:39:20:52 | ... + ... | HTTP url |
36+
| InsecureBasicAuth.java:38:3:38:5 | get | InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" : String | InsecureBasicAuth.java:38:3:38:5 | get | Insecure basic authentication from $@. | InsecureBasicAuth.java:35:19:35:64 | "http://www.example.com:8000/payment/retrieve" | HTTP url |
37+
| InsecureBasicAuth.java:54:3:54:6 | post | InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:54:3:54:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:45:19:45:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
38+
| InsecureBasicAuth.java:71:3:71:6 | post | InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:71:3:71:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:61:19:61:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
39+
| InsecureBasicAuth.java:86:3:86:6 | post | InsecureBasicAuth.java:78:47:78:52 | "http" : String | InsecureBasicAuth.java:86:3:86:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:78:47:78:52 | "http" | HTTP url |
40+
| InsecureBasicAuth.java:102:3:102:6 | post | InsecureBasicAuth.java:93:19:93:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:102:3:102:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:93:19:93:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
41+
| InsecureBasicAuth.java:119:3:119:6 | post | InsecureBasicAuth.java:109:19:109:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:119:3:119:6 | post | Insecure basic authentication from $@. | InsecureBasicAuth.java:109:19:109:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
42+
| InsecureBasicAuth.java:133:3:133:6 | conn | InsecureBasicAuth.java:126:19:126:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" : String | InsecureBasicAuth.java:133:3:133:6 | conn | Insecure basic authentication from $@. | InsecureBasicAuth.java:126:19:126:68 | "http://www.example.com/rest/getuser.do?uid=abcdx" | HTTP url |
43+
| InsecureBasicAuth.java:149:3:149:6 | conn | InsecureBasicAuth.java:145:21:145:28 | protocol : String | InsecureBasicAuth.java:149:3:149:6 | conn | Insecure basic authentication from $@. | InsecureBasicAuth.java:145:21:145:28 | protocol | HTTP url |

0 commit comments

Comments
 (0)