Add files via upload

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.cpp

@@ -0,0 +1,15 @@
+...
+  umask(0); // BAD
+...
+  cmusk = umask(S_IRWXG | S_IRWXO); // GOOD
+  ...
+  fchmod(fileno(fp), 0555 - cmusk); // BAD 
+  ...
+  fchmod(fileno(fp), 0555 & ~curumsk); // GOOD
+...
+  umask(0666);
+  chmod(0666); // BAD
+...
+  umask(0022);
+  chmod(0666); // GOOD
+...

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.qhelp

@@ -0,0 +1,23 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>Finding  for function calls that set file permissions that may have errors in use. Incorrect arithmetic for calculating the resolution mask, using the same mask in opposite functions, using a mask that is too wide.</p>
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed ways to use functions.</p>
+<sample src="IncorrectPrivilegeAssignment.cpp" />
+
+</example>
+<references>
+
+<li>
+  CERT C Coding Standard:
+  <a href="https://wiki.sei.cmu.edu/confluence/display/c/FIO06-C.+Create+files+with+appropriate+access+permissions">FIO06-C. Create files with appropriate access permissions</a>.
+</li>
+
+</references>
+</qhelp>

cpp/ql/src/experimental/Security/CWE/CWE-266/IncorrectPrivilegeAssignment.ql

@@ -0,0 +1,77 @@
+/**
+ * @name Find the wrong use of the umask function.
+ * @description Incorrectly evaluated argument to the umask function may have security implications.
+ * @kind problem
+ * @id cpp/wrong-use-of-the-umask
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       maintainability
+ *       security
+ *       external/cwe/cwe-266
+ *       external/cwe/cwe-264
+ *       external/cwe/cwe-200
+ *       external/cwe/cwe-560
+ *       external/cwe/cwe-687
+ */
+
+import cpp
+import semmle.code.cpp.exprs.BitwiseOperation
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/** Holds for a function `f` that has an argument at index `apos` used to set file permissions. */
+predicate numberArgumentModFunctions(Function f, int apos) {
+  f.hasGlobalOrStdName("umask") and apos = 0
+  or
+  f.hasGlobalOrStdName("fchmod") and apos = 1
+  or
+  f.hasGlobalOrStdName("chmod") and apos = 1
+}
+
+from FunctionCall fc, string msg
+where
+  fc.getTarget().hasGlobalOrStdName("umask") and
+  fc.getArgument(0).getValue() = "0" and
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("umask") and
+    globalValueNumber(fctmp.getArgument(0)) != globalValueNumber(fc.getArgument(0))
+  ) and
+  exists(FunctionCall fctmp |
+    (
+      fctmp.getTarget().hasGlobalOrStdName("fopen") or
+      fctmp.getTarget().hasGlobalOrStdName("open")
+    ) and
+    fctmp.getNumberOfArguments() = 2 and
+    fctmp.getArgument(0).getValue() != "/dev/null"
+  ) and
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("chmod") or
+    fctmp.getTarget().hasGlobalOrStdName("fchmod")
+  ) and
+  msg = "Using umask (0) may not be safe."
+  or
+  fc.getTarget().hasGlobalOrStdName("umask") and
+  exists(FunctionCall fctmp |
+    (
+      fctmp.getTarget().hasGlobalOrStdName("chmod") or
+      fctmp.getTarget().hasGlobalOrStdName("fchmod")
+    ) and
+    (
+      globalValueNumber(fc.getArgument(0)) = globalValueNumber(fctmp.getArgument(1)) and
+      fc.getArgument(0).getValue() != "0"
+    ) and
+    msg = "not use equal argument in umask and " + fctmp.getTarget().getName() + " functions"
+  )
+  or
+  exists(Expr exptmp, int i |
+    numberArgumentModFunctions(fc.getTarget(), i) and
+    not exptmp.getAChild*() instanceof FunctionCall and
+    not exists(SizeofOperator so | exptmp.getAChild*() = so) and
+    not exists(ArrayExpr aetmp | aetmp.getArrayOffset() = exptmp.getAChild*()) and
+    exptmp.getAChild*() instanceof BinaryArithmeticOperation and
+    not exptmp.getAChild*() instanceof BinaryBitwiseOperation and
+    globalValueNumber(exptmp) = globalValueNumber(fc.getArgument(i)) and
+    not exptmp.isConstant() and
+    msg = "Using arithmetic to compute the mask may not be safe."
+  )
+select fc, msg