diff --git a/ql/src/queries/security/cwe-732/WeakFilePermissions.ql b/ql/src/queries/security/cwe-732/WeakFilePermissions.ql index 0ac0b518aa6..d367bede0b2 100644 --- a/ql/src/queries/security/cwe-732/WeakFilePermissions.ql +++ b/ql/src/queries/security/cwe-732/WeakFilePermissions.ql @@ -14,7 +14,6 @@ private import codeql_ruby.dataflow.SSA private import codeql_ruby.dataflow.internal.DataFlowImpl as DataFlow // TODO: account for flows through tuple assignments - /** An expression referencing the File or FileUtils module */ class FileModuleAccess extends Expr { FileModuleAccess() { @@ -29,18 +28,52 @@ class FileModuleAccess extends Expr { } } +bindingset[p] +int world_permission(int p) { result = p % 8 } + +bindingset[p] +int group_permission(int p) { result = (p / 8) % 8 } + +bindingset[p] +string access(int p) { + p % 4 >= 2 and result = "writable" + or + p % 8 in [4, 5] and result = "readable" +} + +bindingset[s] +int parseInt(string s) { + exists(string values, string str | + 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"] and values = "01234567" and str = s.suffix(1) + 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) + ) + ) +} + /** An expression specifing a file permission that allows group/others read or write access */ class PermissivePermissionsExpr extends Expr { + // TODO: non-literal expressions? PermissivePermissionsExpr() { - this.(IntegerLiteral).getValueText().regexpMatch("0[0-7](([2-7].)|.[2-7])") + exists(int perm, string acc | + perm = parseInt(this.(IntegerLiteral).getValueText()) and + (acc = access(world_permission(perm)) or acc = access(group_permission(perm))) + ) 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].*") + this.(StringLiteral).getValueText().regexpMatch(".*[ago][^-=+]*[+=]*[xXst]*[rw].*") } } @@ -82,5 +115,5 @@ class PermissivePermissionsConfig extends DataFlow::Configuration { 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() \ No newline at end of file +select sink.getNode(), source, sink, "Overly permissive mask sets file to $@.", source.getNode(), + source.getNode().toString()