Skip to content

Commit 7c3c1db

Browse files
committed
Use ApiGraphs in WeakFilePermissions query
1 parent 65d9327 commit 7c3c1db

1 file changed

Lines changed: 7 additions & 25 deletions

File tree

ql/src/queries/security/cwe-732/WeakFilePermissions.ql

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -12,22 +12,7 @@
1212
import ruby
1313
import codeql_ruby.DataFlow
1414
import DataFlow::PathGraph
15-
private import codeql_ruby.dataflow.SSA
16-
17-
// TODO: account for flows through tuple assignments
18-
/** An expression referencing the File or FileUtils module */
19-
class FileModuleAccess extends Expr {
20-
FileModuleAccess() {
21-
this.(ConstantAccess).getName() = "File"
22-
or
23-
this.(ConstantAccess).getName() = "FileUtils"
24-
or
25-
exists(FileModuleAccess fma, Ssa::WriteDefinition def |
26-
def.getARead() = this.getAControlFlowNode() and
27-
def.getWriteAccess().getParent().(Assignment).getRightOperand() = fma
28-
)
29-
}
30-
}
15+
import codeql_ruby.ApiGraphs
3116

3217
bindingset[p]
3318
int world_permission(int p) { result = p.bitAnd(7) }
@@ -58,13 +43,12 @@ class PermissivePermissionsExpr extends Expr {
5843
}
5944

6045
/** A permissions argument of a call to a File/FileUtils method that may modify file permissions */
61-
class PermissionArgument extends Expr {
62-
private MethodCall call;
46+
class PermissionArgument extends DataFlow::Node {
47+
private DataFlow::CallNode call;
6348
private string methodName;
6449

6550
PermissionArgument() {
66-
call.getReceiver() instanceof FileModuleAccess and
67-
call.getMethodName() = methodName and
51+
call = API::moduleImport(["File", "FileUtils"]).getAMethodCall(methodName) and
6852
(
6953
methodName in ["chmod", "chmod_R", "lchmod"] and this = call.getArgument(0)
7054
or
@@ -78,7 +62,7 @@ class PermissionArgument extends Expr {
7862
)
7963
}
8064

81-
MethodCall getCall() { result = call }
65+
MethodCall getCall() { result = call.asExpr().getExpr() }
8266
}
8367

8468
class PermissivePermissionsConfig extends DataFlow::Configuration {
@@ -88,14 +72,12 @@ class PermissivePermissionsConfig extends DataFlow::Configuration {
8872
exists(PermissivePermissionsExpr ppe | source.asExpr().getExpr() = ppe)
8973
}
9074

91-
override predicate isSink(DataFlow::Node sink) {
92-
exists(PermissionArgument arg | sink.asExpr().getExpr() = arg)
93-
}
75+
override predicate isSink(DataFlow::Node sink) { sink instanceof PermissionArgument }
9476
}
9577

9678
from
9779
DataFlow::PathNode source, DataFlow::PathNode sink, PermissivePermissionsConfig conf,
9880
PermissionArgument arg
99-
where conf.hasFlowPath(source, sink) and arg = sink.getNode().asExpr().getExpr()
81+
where conf.hasFlowPath(source, sink) and arg = sink.getNode()
10082
select source.getNode(), source, sink, "Overly permissive mask in $@ sets file to $@.",
10183
arg.getCall(), arg.getCall().toString(), source.getNode(), source.getNode().toString()

0 commit comments

Comments
 (0)