-
Notifications
You must be signed in to change notification settings - Fork 2k
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 1 commit
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
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"); | ||
| env.put(Context.SECURITY_AUTHENTICATION, "simple"); | ||
| environment.put(Context.SECURITY_PRINCIPAL, ldapUserName); | ||
| environment.put(Context.SECURITY_CREDENTIALS, password); | ||
| DirContext dirContext = new InitialDirContext(environment); | ||
| return dirContext; | ||
| } | ||
| } | ||
32 changes: 32 additions & 0 deletions
32
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,32 @@ | ||
| <!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> | ||
| CWE: | ||
| <a href="https://cwe.mitre.org/data/definitions/522">CWE-522: Insufficiently Protected Credentials</a> | ||
| <a href="https://cwe.mitre.org/data/definitions/319">CWE-319: Cleartext Transmission of Sensitive Information</a> | ||
| </li> | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| <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 consists of sending the LDAP server the fully qualified DN of the client (user) and the client's clear-text password</a> | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| </li> | ||
| </references> | ||
| </qhelp> | ||
153 changes: 153 additions & 0 deletions
153
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,153 @@ | ||
| /** | ||
| * @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 semmle.code.java.frameworks.Jndi | ||
| import semmle.code.java.dataflow.TaintTracking | ||
| import DataFlow::PathGraph | ||
|
|
||
| /** | ||
| * Gets a regular expression for matching private hosts, which only matches the host portion therefore checking for port is not necessary. | ||
| */ | ||
| private string getPrivateHostRegex() { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| result = | ||
| "(?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\\]?(?:[:/?#].*)?" | ||
| } | ||
|
|
||
| /** | ||
| * String of LDAP connections not in private domains. | ||
| */ | ||
| class LdapStringLiteral extends StringLiteral { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| LdapStringLiteral() { | ||
| // 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 | ||
|
luchua-bc marked this conversation as resolved.
|
||
| not s.substring(7, s.length()).regexpMatch(getPrivateHostRegex()) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** 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 concatLdapString(Expr protocol, Expr host) { | ||
| ( | ||
| protocol.(CompileTimeConstantExpr).getStringValue().regexpMatch("(?i)ldap(://)?") or | ||
| protocol | ||
| .(VarAccess) | ||
| .getVariable() | ||
| .getAnAssignedValue() | ||
| .(CompileTimeConstantExpr) | ||
| .getStringValue() | ||
| .regexpMatch("(?i)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.regexpMatch(getPrivateHostRegex()) | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| ) | ||
| } | ||
|
|
||
| /** 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 `LdapStringLiteral`. | ||
| */ | ||
| class LdapString extends Expr { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| LdapString() { | ||
| this instanceof LdapStringLiteral | ||
| or | ||
| concatLdapString(this.(AddExpr).getLeftOperand(), | ||
| getLeftmostConcatOperand(this.(AddExpr).getRightOperand())) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Tainted value passed to env `Hashtable` as the provider URL, i.e. | ||
| * `env.put(Context.PROVIDER_URL, tainted)` or `env.setProperty(Context.PROVIDER_URL, tainted)`. | ||
| */ | ||
|
luchua-bc marked this conversation as resolved.
|
||
| predicate isProviderUrlEnv(MethodAccess ma) { | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and | ||
| (ma.getMethod().hasName("put") or ma.getMethod().hasName("setProperty")) and | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| ( | ||
| 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.
|
||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if the value "simple" is passed to env `Hashtable` as the authentication mechanism, i.e. | ||
| * `env.put(Context.SECURITY_AUTHENTICATION, "simple")` or `env.setProperty(Context.SECURITY_AUTHENTICATION, "simple")`. | ||
| */ | ||
|
luchua-bc marked this conversation as resolved.
|
||
| predicate isSimpleAuthEnv(MethodAccess ma) { | ||
| ma.getMethod().getDeclaringType().getAnAncestor() instanceof TypeHashtable and | ||
| (ma.getMethod().hasName("put") or ma.getMethod().hasName("setProperty")) and | ||
|
luchua-bc marked this conversation as resolved.
Outdated
|
||
| ( | ||
| ma.getArgument(0).(CompileTimeConstantExpr).getStringValue() = | ||
| "java.naming.security.authentication" | ||
| or | ||
| exists(Field f | | ||
| ma.getArgument(0) = f.getAnAccess() and | ||
| f.hasName("SECURITY_AUTHENTICATION") and | ||
| f.getDeclaringType() instanceof TypeNamingContext | ||
| ) | ||
| ) and | ||
| ma.getArgument(1).(CompileTimeConstantExpr).getStringValue() = "simple" | ||
| } | ||
|
|
||
| /** | ||
| * A taint-tracking configuration for cleartext credentials in LDAP authentication. | ||
| */ | ||
| class LdapAuthFlowConfig extends TaintTracking::Configuration { | ||
| LdapAuthFlowConfig() { this = "InsecureLdapAuth:LdapAuthFlowConfig" } | ||
|
|
||
| /** Source of non-private LDAP connection string */ | ||
| override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof LdapString } | ||
|
|
||
| /** Sink of provider URL with simple authentication */ | ||
| override predicate isSink(DataFlow::Node sink) { | ||
| exists(MethodAccess pma | | ||
| sink.asExpr() = pma.getArgument(1) and | ||
| isProviderUrlEnv(pma) and | ||
| exists(MethodAccess sma | | ||
| sma.getQualifier() = pma.getQualifier().(VarAccess).getVariable().getAnAccess() and | ||
| isSimpleAuthEnv(sma) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| from DataFlow::PathNode source, DataFlow::PathNode sink, LdapAuthFlowConfig config | ||
| where config.hasFlowPath(source, sink) | ||
| select sink.getNode(), source, sink, "Insecure LDAP authentication from $@.", source.getNode(), | ||
| "LDAP connection string" | ||
15 changes: 15 additions & 0 deletions
15
java/ql/test/experimental/query-tests/security/CWE-522/InsecureLdapAuth.expected
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,15 @@ | ||
| edges | ||
| | InsecureLdapAuth.java:11:20:11:50 | "ldap://ad.your-server.com:389" : String | InsecureLdapAuth.java:15:41:15:47 | ldapUrl | | ||
| | InsecureLdapAuth.java:25:20:25:39 | ... + ... : String | InsecureLdapAuth.java:29:41:29:47 | ldapUrl | | ||
| | InsecureLdapAuth.java:81:20:81:50 | "ldap://ad.your-server.com:389" : String | InsecureLdapAuth.java:85:41:85:47 | ldapUrl | | ||
| nodes | ||
| | InsecureLdapAuth.java:11:20:11:50 | "ldap://ad.your-server.com:389" : String | semmle.label | "ldap://ad.your-server.com:389" : String | | ||
| | InsecureLdapAuth.java:15:41:15:47 | ldapUrl | semmle.label | ldapUrl | | ||
| | InsecureLdapAuth.java:25:20:25:39 | ... + ... : String | semmle.label | ... + ... : String | | ||
| | InsecureLdapAuth.java:29:41:29:47 | ldapUrl | semmle.label | ldapUrl | | ||
| | InsecureLdapAuth.java:81:20:81:50 | "ldap://ad.your-server.com:389" : String | semmle.label | "ldap://ad.your-server.com:389" : String | | ||
| | InsecureLdapAuth.java:85:41:85:47 | ldapUrl | semmle.label | ldapUrl | | ||
| #select | ||
| | InsecureLdapAuth.java:15:41:15:47 | ldapUrl | InsecureLdapAuth.java:11:20:11:50 | "ldap://ad.your-server.com:389" : String | InsecureLdapAuth.java:15:41:15:47 | ldapUrl | Insecure LDAP authentication from $@. | InsecureLdapAuth.java:11:20:11:50 | "ldap://ad.your-server.com:389" | LDAP connection string | | ||
| | InsecureLdapAuth.java:29:41:29:47 | ldapUrl | InsecureLdapAuth.java:25:20:25:39 | ... + ... : String | InsecureLdapAuth.java:29:41:29:47 | ldapUrl | Insecure LDAP authentication from $@. | InsecureLdapAuth.java:25:20:25:39 | ... + ... | LDAP connection string | | ||
| | InsecureLdapAuth.java:85:41:85:47 | ldapUrl | InsecureLdapAuth.java:81:20:81:50 | "ldap://ad.your-server.com:389" : String | InsecureLdapAuth.java:85:41:85:47 | ldapUrl | Insecure LDAP authentication from $@. | InsecureLdapAuth.java:81:20:81:50 | "ldap://ad.your-server.com:389" | LDAP connection string | |
92 changes: 92 additions & 0 deletions
92
java/ql/test/experimental/query-tests/security/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,92 @@ | ||
| import java.util.Hashtable; | ||
|
|
||
| import javax.naming.Context; | ||
| import javax.naming.directory.DirContext; | ||
| import javax.naming.directory.InitialDirContext; | ||
| import javax.naming.ldap.InitialLdapContext; | ||
|
|
||
| public class InsecureLdapAuth { | ||
| // BAD - Test LDAP authentication in cleartext using `DirContext`. | ||
| public void testCleartextLdapAuth(String ldapUserName, String password) { | ||
|
luchua-bc marked this conversation as resolved.
|
||
| String ldapUrl = "ldap://ad.your-server.com:389"; | ||
| 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); | ||
| } | ||
|
|
||
| // BAD - Test LDAP authentication in cleartext using `DirContext`. | ||
| public void testCleartextLdapAuth(String ldapUserName, String password, String serverName) { | ||
| String ldapUrl = "ldap://"+serverName+":389"; | ||
| 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); | ||
| } | ||
|
|
||
| // GOOD - Test LDAP authentication over SSL. | ||
| public void testSslLdapAuth(String ldapUserName, String password) { | ||
| 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); | ||
| } | ||
|
|
||
| // GOOD - Test LDAP authentication with SASL authentication. | ||
| public void testSaslLdapAuth(String ldapUserName, String password) { | ||
| String ldapUrl = "ldap://ad.your-server.com:389"; | ||
| 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, "DIGEST-MD5 GSSAPI"); | ||
| environment.put(Context.SECURITY_PRINCIPAL, ldapUserName); | ||
| environment.put(Context.SECURITY_CREDENTIALS, password); | ||
| DirContext dirContext = new InitialDirContext(environment); | ||
| } | ||
|
|
||
| // GOOD - Test LDAP authentication in cleartext connecting to local LDAP server. | ||
| public void testCleartextLdapAuth2(String ldapUserName, String password) { | ||
| String ldapUrl = "ldap://localhost:389"; | ||
| 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); | ||
| } | ||
|
|
||
| // BAD - Test LDAP authentication in cleartext using `InitialLdapContext`. | ||
| public void testCleartextLdapAuth3(String ldapUserName, String password) { | ||
| String ldapUrl = "ldap://ad.your-server.com:389"; | ||
| 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); | ||
| InitialLdapContext ldapContext = new InitialLdapContext(environment, null); | ||
| } | ||
| } | ||
1 change: 1 addition & 0 deletions
1
java/ql/test/experimental/query-tests/security/CWE-522/InsecureLdapAuth.qlref
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 @@ | ||
| experimental/Security/CWE/CWE-522/InsecureLdapAuth.ql |
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.