Skip to content

Commit 57cf5f1

Browse files
authored
Fix compilation error and review issues in Java security queries
- Replace getAValue() with getValue() in InsecureDirectObjectReference.ql to compile against codeql/java-all@7.1.3 (CI uses CLI 2.21.1) - Fix getAStringArrayValue -> getAStringValue for PathVariable/RequestParam annotations (value/name are single String attrs, not arrays) - Remove setUrls from LdapUrlSink (takes String[], not a single constant) - Remove LDAP URL literal from alert message to avoid exposing credentials - Improve InsecureDirectObjectReference alert message clarity
1 parent c5361b0 commit 57cf5f1

2 files changed

Lines changed: 5 additions & 5 deletions

File tree

java/src/security/CWE-319/CleartextLdapUrl.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class LdapUrlSink extends MethodCall {
3232
Expr urlArg;
3333

3434
LdapUrlSink() {
35-
this.getMethod().hasName(["setUrl", "setUrls", "setProviderUrl"]) and
35+
this.getMethod().hasName(["setUrl", "setProviderUrl"]) and
3636
this.getQualifier().getType().(RefType).getName().matches("%ContextSource%") and
3737
urlArg = this.getArgument(0)
3838
}
@@ -44,4 +44,4 @@ from LdapUrlSink sink, string url
4444
where
4545
constantStringValue(sink.getUrlArg(), url) and
4646
url.regexpMatch("(?i)ldap://.*")
47-
select sink, "LDAP context configured with cleartext URL '" + url + "'; use ldaps:// or STARTTLS."
47+
select sink, "LDAP context configured with a cleartext ldap:// URL; use ldaps:// or STARTTLS."

java/src/security/CWE-639/InsecureDirectObjectReference.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class ActionMethod extends SpringRequestMappingMethod {
2424
/** Gets a string describing this action: method name, class name, route, or HTTP verb. */
2525
string getADescription() {
2626
result =
27-
[this.getName(), this.getDeclaringType().getName(), this.getAValue(), this.getMethodValue()]
27+
[this.getName(), this.getDeclaringType().getName(), this.getValue(), this.getMethodValue()]
2828
}
2929

3030
/** Holds if this action may represent a state-changing operation. */
@@ -64,7 +64,7 @@ predicate hasIdParameter(ActionMethod m) {
6464
a.getType()
6565
.hasQualifiedName("org.springframework.web.bind.annotation",
6666
["PathVariable", "RequestParam"]) and
67-
a.getAStringArrayValue(["value", "name"]).toLowerCase().matches(["%id", "%idx"])
67+
a.getAStringValue(["value", "name"]).toLowerCase().matches(["%id", "%idx"])
6868
)
6969
)
7070
}
@@ -121,4 +121,4 @@ predicate hasInsecureDirectObjectReference(ActionMethod m) {
121121
from ActionMethod m
122122
where hasInsecureDirectObjectReference(m)
123123
select m,
124-
"This action may be missing authorization checks for which users can access the resource of the provided id."
124+
"This action may be missing authorization checks to verify the current user is permitted to access the resource identified by the provided id."

0 commit comments

Comments
 (0)