diff --git a/ql/src/codeql_ruby/ast/Literal.qll b/ql/src/codeql_ruby/ast/Literal.qll index ce0825cce92..6a6cfc9ab5c 100644 --- a/ql/src/codeql_ruby/ast/Literal.qll +++ b/ql/src/codeql_ruby/ast/Literal.qll @@ -47,6 +47,42 @@ class IntegerLiteral extends NumericLiteral, TIntegerLiteral { final override string getValueText() { result = g.getValue() } + /** Gets the numerical value of this integer literal. */ + final int getValue() { + exists(string s, string values, string str | + s = this.getValueText().toLowerCase() and + ( + s.matches("0b%") and + values = "01" and + str = s.suffix(2) + or + s.matches("0x%") and + values = "0123456789abcdef" and + str = s.suffix(2) + or + s.charAt(0) = "0" and + not s.charAt(1) = ["b", "x", "o"] and + values = "01234567" and + str = s.suffix(1) + or + s.matches("0o%") and + values = "01234567" and + str = s.suffix(2) + or + s.charAt(0) != "0" and values = "0123456789" and str = s + ) + | + result = + sum(int index, string c, int v, int exp | + c = str.replaceAll("_", "").charAt(index) and + v = values.indexOf(c.toLowerCase()) and + exp = str.replaceAll("_", "").length() - index - 1 + | + v * values.length().pow(exp) + ) + ) + } + final override string toString() { result = this.getValueText() } final override string getAPrimaryQlClass() { result = "IntegerLiteral" } diff --git a/ql/src/queries/security/cwe-732/WeakFilePermissions.qhelp b/ql/src/queries/security/cwe-732/WeakFilePermissions.qhelp new file mode 100644 index 00000000000..a5f8997e911 --- /dev/null +++ b/ql/src/queries/security/cwe-732/WeakFilePermissions.qhelp @@ -0,0 +1,26 @@ + + + + +

+When creating a file, POSIX systems allow permissions to be specified +for owner, group and others separately. Permissions should be kept as +strict as possible, preventing access to the files contents by other users. +

+ +
+ + +

+Restrict the file permissions of files to prevent any but the owner being able to read or write to that file +

+
+ + +
  • +Wikipedia: +File system permissions. +
  • +
    + +
    diff --git a/ql/src/queries/security/cwe-732/WeakFilePermissions.ql b/ql/src/queries/security/cwe-732/WeakFilePermissions.ql new file mode 100644 index 00000000000..41c0edceb16 --- /dev/null +++ b/ql/src/queries/security/cwe-732/WeakFilePermissions.ql @@ -0,0 +1,99 @@ +/** + * @name Overly permissive file permissions + * @description Allowing files to be readable or writable by users other than the owner may allow sensitive information to be accessed. + * @kind path-problem + * @problem.severity warning + * @id rb/overly-permissive-file + * @tags external/cwe/cwe-732 + * security + * @precision low + */ + +import ruby +import codeql_ruby.DataFlow +import DataFlow::PathGraph +private import codeql_ruby.dataflow.SSA + +// TODO: account for flows through tuple assignments +/** An expression referencing the File or FileUtils module */ +class FileModuleAccess extends Expr { + FileModuleAccess() { + this.(ConstantAccess).getName() = "File" + or + this.(ConstantAccess).getName() = "FileUtils" + or + exists(FileModuleAccess fma, Ssa::WriteDefinition def | + def.getARead() = this.getAControlFlowNode() and + def.getWriteAccess().getParent().(Assignment).getRightOperand() = fma + ) + } +} + +bindingset[p] +int world_permission(int p) { result = p.bitAnd(7) } + +// 70 oct = 56 dec +bindingset[p] +int group_permission(int p) { result = p.bitAnd(56) } + +bindingset[p] +string access(int p) { + p.bitAnd(2) != 0 and result = "writable" + or + p.bitAnd(4) != 0 and result = "readable" +} + +/** An expression specifing a file permission that allows group/others read or write access */ +class PermissivePermissionsExpr extends Expr { + // TODO: non-literal expressions? + PermissivePermissionsExpr() { + exists(int perm, string acc | + perm = this.(IntegerLiteral).getValue() and + (acc = access(world_permission(perm)) or acc = access(group_permission(perm))) + ) + or + // adding/setting read or write permissions for all/group/other + this.(StringLiteral).getValueText().regexpMatch(".*[ago][^-=+]*[+=][xXst]*[rw].*") + } +} + +/** A call to a method of File or FileUtils that may modify file permissions */ +class PermissionSettingMethodCall extends MethodCall { + private string methodName; + private Expr permArg; + + PermissionSettingMethodCall() { + this.getReceiver() instanceof FileModuleAccess and + this.getMethodName() = methodName and + ( + methodName in ["chmod", "chmod_R", "lchmod"] and permArg = this.getArgument(0) + or + methodName = "mkfifo" and permArg = this.getArgument(1) + or + methodName in ["new", "open"] and permArg = this.getArgument(2) + or + methodName in ["install", "makedirs", "mkdir", "mkdir_p", "mkpath"] and + permArg = this.getKeywordArgument("mode") + // TODO: defaults for optional args? This may depend on the umask + ) + } + + Expr getPermissionArgument() { result = permArg } +} + +class PermissivePermissionsConfig extends DataFlow::Configuration { + PermissivePermissionsConfig() { this = "PermissivePermissionsConfig" } + + override predicate isSource(DataFlow::Node source) { + exists(PermissivePermissionsExpr ppe | source.asExpr().getExpr() = ppe) + } + + override predicate isSink(DataFlow::Node sink) { + exists(PermissionSettingMethodCall c | sink.asExpr().getExpr() = c.getPermissionArgument()) + } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, PermissivePermissionsConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Overly permissive mask sets file to $@.", source.getNode(), + source.getNode().toString() diff --git a/ql/test/query-tests/security/cwe-732/FilePermissions.rb b/ql/test/query-tests/security/cwe-732/FilePermissions.rb new file mode 100644 index 00000000000..7edb567a8e6 --- /dev/null +++ b/ql/test/query-tests/security/cwe-732/FilePermissions.rb @@ -0,0 +1,58 @@ +require "fileutils" + +def run_chmod_1(filename) + FileUtils.chmod 0222, filename + FileUtils.chmod 0622, filename + FileUtils.chmod 0755, filename + FileUtils.chmod 0777, filename +end + +module DummyModule + def chmod(mode, list, options = {} ) + list + end +end + +def run_chmod_2(filename) + foo = FileUtils + bar = foo + baz = Dummy + # "safe" + baz.chmod 0755, filename + baz = bar + # unsafe + baz.chmod 0755, filename +end + +def run_chmod_3(filename) + # TODO: we currently miss this + foo = FileUtils + bar, baz = foo, 7 + bar.chmod 0755, filename +end + +def run_chmod_4(filename) + # safe permissions + FileUtils.chmod 0700, filename + FileUtils.chmod 0711, filename + FileUtils.chmod 0701, filename + FileUtils.chmod 0710, filename +end + +def run_chmod_5(filename) + perm = 0777 + FileUtils.chmod perm, filename + perm2 = perm + FileUtils.chmod perm2, filename + + perm = "u=wrx,g=rwx,o=x" + perm2 = perm + FileUtils.chmod perm2, filename + FileUtils.chmod "u=rwx,o+r", filename + FileUtils.chmod "u=rwx,go-r", filename + FileUtils.chmod "a+rw", filename +end + +def run_chmod_R(filename) + File.chmod_R 0755, filename +end diff --git a/ql/test/query-tests/security/cwe-732/WeakFilePermissions.expected b/ql/test/query-tests/security/cwe-732/WeakFilePermissions.expected new file mode 100644 index 00000000000..965851448ff --- /dev/null +++ b/ql/test/query-tests/security/cwe-732/WeakFilePermissions.expected @@ -0,0 +1,30 @@ +edges +| FilePermissions.rb:43:10:43:13 | 0777 : | FilePermissions.rb:44:19:44:22 | perm | +| FilePermissions.rb:43:10:43:13 | 0777 : | FilePermissions.rb:46:19:46:23 | perm2 | +| FilePermissions.rb:48:10:48:26 | "u=wrx,g=rwx,o=x" : | FilePermissions.rb:50:19:50:23 | perm2 | +nodes +| FilePermissions.rb:4:19:4:22 | 0222 | semmle.label | 0222 | +| FilePermissions.rb:5:19:5:22 | 0622 | semmle.label | 0622 | +| FilePermissions.rb:6:19:6:22 | 0755 | semmle.label | 0755 | +| FilePermissions.rb:7:19:7:22 | 0777 | semmle.label | 0777 | +| FilePermissions.rb:24:13:24:16 | 0755 | semmle.label | 0755 | +| FilePermissions.rb:43:10:43:13 | 0777 : | semmle.label | 0777 : | +| FilePermissions.rb:44:19:44:22 | perm | semmle.label | perm | +| FilePermissions.rb:46:19:46:23 | perm2 | semmle.label | perm2 | +| FilePermissions.rb:48:10:48:26 | "u=wrx,g=rwx,o=x" : | semmle.label | "u=wrx,g=rwx,o=x" : | +| FilePermissions.rb:50:19:50:23 | perm2 | semmle.label | perm2 | +| FilePermissions.rb:51:19:51:29 | "u=rwx,o+r" | semmle.label | "u=rwx,o+r" | +| FilePermissions.rb:53:19:53:24 | "a+rw" | semmle.label | "a+rw" | +| FilePermissions.rb:57:16:57:19 | 0755 | semmle.label | 0755 | +#select +| FilePermissions.rb:4:19:4:22 | 0222 | FilePermissions.rb:4:19:4:22 | 0222 | FilePermissions.rb:4:19:4:22 | 0222 | Overly permissive mask sets file to $@. | FilePermissions.rb:4:19:4:22 | 0222 | 0222 | +| FilePermissions.rb:5:19:5:22 | 0622 | FilePermissions.rb:5:19:5:22 | 0622 | FilePermissions.rb:5:19:5:22 | 0622 | Overly permissive mask sets file to $@. | FilePermissions.rb:5:19:5:22 | 0622 | 0622 | +| FilePermissions.rb:6:19:6:22 | 0755 | FilePermissions.rb:6:19:6:22 | 0755 | FilePermissions.rb:6:19:6:22 | 0755 | Overly permissive mask sets file to $@. | FilePermissions.rb:6:19:6:22 | 0755 | 0755 | +| FilePermissions.rb:7:19:7:22 | 0777 | FilePermissions.rb:7:19:7:22 | 0777 | FilePermissions.rb:7:19:7:22 | 0777 | Overly permissive mask sets file to $@. | FilePermissions.rb:7:19:7:22 | 0777 | 0777 | +| FilePermissions.rb:24:13:24:16 | 0755 | FilePermissions.rb:24:13:24:16 | 0755 | FilePermissions.rb:24:13:24:16 | 0755 | Overly permissive mask sets file to $@. | FilePermissions.rb:24:13:24:16 | 0755 | 0755 | +| FilePermissions.rb:44:19:44:22 | perm | FilePermissions.rb:43:10:43:13 | 0777 : | FilePermissions.rb:44:19:44:22 | perm | Overly permissive mask sets file to $@. | FilePermissions.rb:43:10:43:13 | 0777 | 0777 | +| FilePermissions.rb:46:19:46:23 | perm2 | FilePermissions.rb:43:10:43:13 | 0777 : | FilePermissions.rb:46:19:46:23 | perm2 | Overly permissive mask sets file to $@. | FilePermissions.rb:43:10:43:13 | 0777 | 0777 | +| FilePermissions.rb:50:19:50:23 | perm2 | FilePermissions.rb:48:10:48:26 | "u=wrx,g=rwx,o=x" : | FilePermissions.rb:50:19:50:23 | perm2 | Overly permissive mask sets file to $@. | FilePermissions.rb:48:10:48:26 | "u=wrx,g=rwx,o=x" | "u=wrx,g=rwx,o=x" | +| FilePermissions.rb:51:19:51:29 | "u=rwx,o+r" | FilePermissions.rb:51:19:51:29 | "u=rwx,o+r" | FilePermissions.rb:51:19:51:29 | "u=rwx,o+r" | Overly permissive mask sets file to $@. | FilePermissions.rb:51:19:51:29 | "u=rwx,o+r" | "u=rwx,o+r" | +| FilePermissions.rb:53:19:53:24 | "a+rw" | FilePermissions.rb:53:19:53:24 | "a+rw" | FilePermissions.rb:53:19:53:24 | "a+rw" | Overly permissive mask sets file to $@. | FilePermissions.rb:53:19:53:24 | "a+rw" | "a+rw" | +| FilePermissions.rb:57:16:57:19 | 0755 | FilePermissions.rb:57:16:57:19 | 0755 | FilePermissions.rb:57:16:57:19 | 0755 | Overly permissive mask sets file to $@. | FilePermissions.rb:57:16:57:19 | 0755 | 0755 | diff --git a/ql/test/query-tests/security/cwe-732/WeakFilePermissions.qlref b/ql/test/query-tests/security/cwe-732/WeakFilePermissions.qlref new file mode 100644 index 00000000000..bf19b31509d --- /dev/null +++ b/ql/test/query-tests/security/cwe-732/WeakFilePermissions.qlref @@ -0,0 +1 @@ +queries/security/cwe-732/WeakFilePermissions.ql