WIP rb/overly-permissive-file query

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

@@ -0,0 +1,79 @@
+/**
+ * @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 problem
+ * @problem.severity warning
+ * @id rb/overly-permissive-file
+ * @tags external/cwe/cwe-732
+ *       security
+ * @precision low
+ */
+
+import ruby
+private import codeql_ruby.dataflow.SSA
+
+// TODO: account for flows through tuple assignments
+// TODO: full dataflow?
+/** 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
+    )
+  }
+}
+
+/** An expression specifing a file permission that allows group/others read or write access */
+class PermissivePermissionsExpr extends Expr {
+  PermissivePermissionsExpr() {
+    this.(IntegerLiteral).getValueText().regexpMatch("0[0-7](([2-7].)|.[2-7])")
+    or
+    this.(IntegerLiteral)
+        .getValueText()
+        .regexpMatch("0b[01]{3}+((1[01]{5}+)|([01]1[01]{4}+)|([01]{3}+1[01]{2}+)|([01]{4}+1[01]))")
+    or
+    // TODO: non-literal expressions? underscores? decimal/hex literals?
+    // adding/setting read or write permissions for all/group/owner
+    this.(StringLiteral).getValueText().regexpMatch(".*[ago][^-=+]*[+=]*[rwxXst]*[rw].*")
+    or
+    exists(PermissivePermissionsExpr ppe, Ssa::WriteDefinition def |
+      def.getARead() = this.getAControlFlowNode() and
+      def.getWriteAccess().getParent().(Assignment).getRightOperand() = ppe
+    )
+  }
+}
+
+/** 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 }
+
+  predicate isPermissive() { this.getPermissionArgument() instanceof PermissivePermissionsExpr }
+}
+
+from PermissionSettingMethodCall c
+where c.isPermissive()
+select c, "Overly permissive mask sets file to " + c.getPermissionArgument() + "."

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

ql/test/query-tests/security/cwe-732/WeakFilePermissions.expected

@@ -0,0 +1,11 @@
+| FilePermissions.rb:4:3:4:32 | call to chmod | Overly permissive mask sets file to 0222. |
+| FilePermissions.rb:5:3:5:32 | call to chmod | Overly permissive mask sets file to 0622. |
+| FilePermissions.rb:6:3:6:32 | call to chmod | Overly permissive mask sets file to 0755. |
+| FilePermissions.rb:7:3:7:32 | call to chmod | Overly permissive mask sets file to 0777. |
+| FilePermissions.rb:24:3:24:26 | call to chmod | Overly permissive mask sets file to 0755. |
+| FilePermissions.rb:44:3:44:32 | call to chmod | Overly permissive mask sets file to perm. |
+| FilePermissions.rb:46:3:46:33 | call to chmod | Overly permissive mask sets file to perm2. |
+| FilePermissions.rb:50:3:50:33 | call to chmod | Overly permissive mask sets file to perm2. |
+| FilePermissions.rb:51:3:51:39 | call to chmod | Overly permissive mask sets file to "u=rwx,o+r". |
+| FilePermissions.rb:53:3:53:34 | call to chmod | Overly permissive mask sets file to "a+rw". |
+| FilePermissions.rb:57:3:57:29 | call to chmod_R | Overly permissive mask sets file to 0755. |

ql/test/query-tests/security/cwe-732/WeakFilePermissions.qlref

@@ -0,0 +1 @@
+queries/security/cwe-732/WeakFilePermissions.ql