-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Java: Insecure LDAP authentication #4854
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
4ec78d0
Insecure LDAP authentication
luchua-bc c069a5b
Factor private host regex into the networking library and enhance the…
luchua-bc babe744
Add SECURITY_PROTOCOL check
luchua-bc e5a703e
Revamp the query
luchua-bc 32c5462
Drop fieldName from the function for runtime evaluation
luchua-bc 6a93099
Simplify the query and update qldoc
luchua-bc 3151aef
Enhance the query
luchua-bc 2ace10f
Use PostUpdateNode for wrapper method calls
luchua-bc 724c3e0
Update help file
luchua-bc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
24 changes: 24 additions & 0 deletions
24
java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
public class InsecureLdapAuth { | ||
/** LDAP authentication */ | ||
public DirContext ldapAuth(String ldapUserName, String password) { | ||
{ | ||
// BAD: LDAP authentication in cleartext | ||
String ldapUrl = "ldap://ad.your-server.com:389"; | ||
} | ||
|
||
{ | ||
// GOOD: LDAPS authentication over SSL | ||
String ldapUrl = "ldaps://ad.your-server.com:636"; | ||
} | ||
|
||
Hashtable<String, String> environment = new Hashtable<String, String>(); | ||
environment.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); | ||
environment.put(Context.PROVIDER_URL, ldapUrl); | ||
environment.put(Context.REFERRAL, "follow"); | ||
environment.put(Context.SECURITY_AUTHENTICATION, "simple"); | ||
environment.put(Context.SECURITY_PRINCIPAL, ldapUserName); | ||
environment.put(Context.SECURITY_CREDENTIALS, password); | ||
DirContext dirContext = new InitialDirContext(environment); | ||
return dirContext; | ||
} | ||
} |
27 changes: 27 additions & 0 deletions
27
java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.qhelp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd"> | ||
<qhelp> | ||
|
||
<overview> | ||
<p>When using the Java LDAP API to perform LDAPv3-style extended operations and controls, a context with connection properties including user credentials is started. Transmission of LDAP credentials in cleartext allows remote attackers to obtain sensitive information by sniffing the network.</p> | ||
</overview> | ||
|
||
<recommendation> | ||
<p>Use LDAPS to send credentials through SSL or use SASL authentication.</p> | ||
</recommendation> | ||
|
||
<example> | ||
<p>The following example shows two ways of using LDAP authentication. In the 'BAD' case, the credentials are transmitted in cleartext. In the 'GOOD' case, the credentials are transmitted over SSL.</p> | ||
<sample src="InsecureLdapAuth.java" /> | ||
</example> | ||
|
||
<references> | ||
<li> | ||
Oracle: | ||
<a href="https://docs.oracle.com/javase/jndi/tutorial/ldap/misc/url.html">LDAP and LDAPS URLs</a> | ||
</li> | ||
<li> | ||
Oracle: | ||
<a href="https://docs.oracle.com/javase/tutorial/jndi/ldap/simple.html">Simple authentication</a> | ||
</li> | ||
</references> | ||
</qhelp> |
213 changes: 213 additions & 0 deletions
213
java/ql/src/experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,213 @@ | ||
/** | ||
* @name Insecure LDAP authentication | ||
* @description LDAP authentication with credentials sent in cleartext. | ||
* @kind path-problem | ||
* @id java/insecure-ldap-auth | ||
* @tags security | ||
* external/cwe-522 | ||
* external/cwe-319 | ||
*/ | ||
|
||
import java | ||
import DataFlow | ||
import semmle.code.java.frameworks.Jndi | ||
import semmle.code.java.frameworks.Networking | ||
import semmle.code.java.dataflow.TaintTracking | ||
import DataFlow::PathGraph | ||
|
||
/** | ||
* Insecure (non-SSL, non-private) LDAP URL string literal. | ||
*/ | ||
class InsecureLdapUrlLiteral extends StringLiteral { | ||
InsecureLdapUrlLiteral() { | ||
// Match connection strings with the LDAP protocol and without private IP addresses to reduce false positives. | ||
exists(string s | this.getRepresentedString() = s | | ||
s.regexpMatch("(?i)ldap://[\\[a-zA-Z0-9].*") and | ||
not s.substring(7, s.length()) instanceof PrivateHostName | ||
) | ||
} | ||
} | ||
|
||
/** The interface `javax.naming.Context`. */ | ||
class TypeNamingContext extends Interface { | ||
TypeNamingContext() { this.hasQualifiedName("javax.naming", "Context") } | ||
} | ||
|
||
/** The class `java.util.Hashtable`. */ | ||
class TypeHashtable extends Class { | ||
TypeHashtable() { this.getSourceDeclaration().hasQualifiedName("java.util", "Hashtable") } | ||
} | ||
|
||
/** | ||
* Holds if a non-private LDAP string is concatenated from both protocol and host. | ||
*/ | ||
predicate concatInsecureLdapString(Expr protocol, Expr host) { | ||
protocol.(CompileTimeConstantExpr).getStringValue() = "ldap://" and | ||
not exists(string hostString | | ||
hostString = host.(CompileTimeConstantExpr).getStringValue() or | ||
hostString = | ||
host.(VarAccess).getVariable().getAnAssignedValue().(CompileTimeConstantExpr).getStringValue() | ||
| | ||
hostString.length() = 0 or // Empty host is loopback address | ||
hostString instanceof PrivateHostName | ||
) | ||
} | ||
|
||
/** Gets the leftmost operand in a concatenated string */ | ||
Expr getLeftmostConcatOperand(Expr expr) { | ||
if expr instanceof AddExpr | ||
then result = getLeftmostConcatOperand(expr.(AddExpr).getLeftOperand()) | ||
else result = expr | ||
} | ||
|
||
/** | ||
* String concatenated with `InsecureLdapUrlLiteral`. | ||
*/ | ||
class InsecureLdapUrl extends Expr { | ||
InsecureLdapUrl() { | ||
this instanceof InsecureLdapUrlLiteral | ||
or | ||
concatInsecureLdapString(this.(AddExpr).getLeftOperand(), | ||
getLeftmostConcatOperand(this.(AddExpr).getRightOperand())) | ||
} | ||
} | ||
|
||
/** | ||
* Holds if `ma` writes the `java.naming.provider.url` (also known as `Context.PROVIDER_URL`) key of a `Hashtable`. | ||
*/ | ||
predicate isProviderUrlSetter(MethodAccess ma) { | ||
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and | ||
ma.getMethod().hasName(["put", "setProperty"]) and | ||
( | ||
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = "java.naming.provider.url" | ||
or | ||
exists(Field f | | ||
ma.getArgument(0) = f.getAnAccess() and | ||
f.hasName("PROVIDER_URL") and | ||
f.getDeclaringType() instanceof TypeNamingContext | ||
) | ||
luchua-bc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
} | ||
|
||
/** | ||
* Holds if `ma` sets `fieldValue` to `envValue` in some `Hashtable`. | ||
*/ | ||
bindingset[fieldValue, envValue] | ||
predicate hasFieldValueEnv(MethodAccess ma, string fieldValue, string envValue) { | ||
// environment.put("java.naming.security.authentication", "simple") | ||
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and | ||
ma.getMethod().hasName(["put", "setProperty"]) and | ||
ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = fieldValue and | ||
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue | ||
} | ||
|
||
/** | ||
* Holds if `ma` sets attribute name `fieldName` to `envValue` in some `Hashtable`. | ||
*/ | ||
bindingset[fieldName, envValue] | ||
predicate hasFieldNameEnv(MethodAccess ma, string fieldName, string envValue) { | ||
// environment.put(Context.SECURITY_AUTHENTICATION, "simple") | ||
ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and | ||
ma.getMethod().hasName(["put", "setProperty"]) and | ||
exists(Field f | | ||
ma.getArgument(0) = f.getAnAccess() and | ||
f.hasName(fieldName) and | ||
f.getDeclaringType() instanceof TypeNamingContext | ||
) and | ||
ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = envValue | ||
} | ||
|
||
/** | ||
* Holds if `ma` sets `java.naming.security.authentication` (also known as `Context.SECURITY_AUTHENTICATION`) to `simple` in some `Hashtable`. | ||
*/ | ||
predicate isBasicAuthEnv(MethodAccess ma) { | ||
hasFieldValueEnv(ma, "java.naming.security.authentication", "simple") or | ||
hasFieldNameEnv(ma, "SECURITY_AUTHENTICATION", "simple") | ||
} | ||
|
||
/** | ||
* Holds if `ma` sets `java.naming.security.protocol` (also known as `Context.SECURITY_PROTOCOL`) to `ssl` in some `Hashtable`. | ||
*/ | ||
predicate isSSLEnv(MethodAccess ma) { | ||
hasFieldValueEnv(ma, "java.naming.security.protocol", "ssl") or | ||
hasFieldNameEnv(ma, "SECURITY_PROTOCOL", "ssl") | ||
} | ||
|
||
/** | ||
* A taint-tracking configuration for `ldap://` URL in LDAP authentication. | ||
*/ | ||
class InsecureUrlFlowConfig extends TaintTracking::Configuration { | ||
luchua-bc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
InsecureUrlFlowConfig() { this = "InsecureLdapAuth:InsecureUrlFlowConfig" } | ||
|
||
/** Source of `ldap://` connection string. */ | ||
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof InsecureLdapUrl } | ||
|
||
/** Sink of directory context creation. */ | ||
override predicate isSink(DataFlow::Node sink) { | ||
exists(ConstructorCall cc | | ||
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and | ||
sink.asExpr() = cc.getArgument(0) | ||
) | ||
} | ||
|
||
/** Method call of `env.put()`. */ | ||
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) { | ||
exists(MethodAccess ma | | ||
pred.asExpr() = ma.getArgument(1) and | ||
isProviderUrlSetter(ma) and | ||
succ.asExpr() = ma.getQualifier() | ||
) | ||
} | ||
} | ||
|
||
/** | ||
* A taint-tracking configuration for `simple` basic-authentication in LDAP configuration. | ||
*/ | ||
class BasicAuthFlowConfig extends DataFlow::Configuration { | ||
BasicAuthFlowConfig() { this = "InsecureLdapAuth:BasicAuthFlowConfig" } | ||
|
||
/** Source of `simple` configuration. */ | ||
override predicate isSource(DataFlow::Node src) { | ||
exists(MethodAccess ma | | ||
isBasicAuthEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr() | ||
) | ||
} | ||
|
||
/** Sink of directory context creation. */ | ||
override predicate isSink(DataFlow::Node sink) { | ||
exists(ConstructorCall cc | | ||
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and | ||
sink.asExpr() = cc.getArgument(0) | ||
) | ||
} | ||
} | ||
|
||
/** | ||
* A taint-tracking configuration for `ssl` configuration in LDAP authentication. | ||
*/ | ||
class SSLFlowConfig extends DataFlow::Configuration { | ||
SSLFlowConfig() { this = "InsecureLdapAuth:SSLFlowConfig" } | ||
|
||
/** Source of `ssl` configuration. */ | ||
override predicate isSource(DataFlow::Node src) { | ||
exists(MethodAccess ma | | ||
isSSLEnv(ma) and ma.getQualifier() = src.(PostUpdateNode).getPreUpdateNode().asExpr() | ||
) | ||
} | ||
|
||
/** Sink of directory context creation. */ | ||
override predicate isSink(DataFlow::Node sink) { | ||
exists(ConstructorCall cc | | ||
cc.getConstructedType().getASupertype*() instanceof TypeDirContext and | ||
sink.asExpr() = cc.getArgument(0) | ||
) | ||
} | ||
} | ||
|
||
from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureUrlFlowConfig config | ||
where | ||
config.hasFlowPath(source, sink) and | ||
exists(BasicAuthFlowConfig bc | bc.hasFlowTo(sink.getNode())) and | ||
not exists(SSLFlowConfig sc | sc.hasFlowTo(sink.getNode())) | ||
select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(), | ||
"LDAP connection string" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.