Merge pull request github#6950 from ihsinme/ihsinme-patch-078

CPP: Add query for CWE-200 Exposure of Sensitive Information to an Unauthorized Actor

Author: geoffw0

cpp/ql/src/experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.cpp

@@ -0,0 +1,10 @@
+...
+  FILE *fp = fopen(filename,"w"); // BAD
+...
+  umask(S_IXUSR|S_IRWXG|S_IRWXO);
+  FILE *fp;
+  fp = fopen(filename,"w"); // GOOD
+  chmod(filename,S_IRUSR|S_IWUSR);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+...

cpp/ql/src/experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.qhelp

@@ -0,0 +1,24 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+<overview>
+<p>When creating a file using a library function such as <code>fopen</code>, the access rights for the newly created file are not specified as part of the call. Instead these rights are determined by the system unless the programmer takes specific measures, such as calling the Posix <code>umask</code> function at some point before the call to <code>fopen</code>. For some applications, the default access rights assigned by the system are not sufficient to protect a file against access by an attacker.</p>
+
+
+</overview>
+
+<example>
+<p>The following example demonstrates erroneous and fixed methods for working with files.</p>
+<sample src="ExposureSensitiveInformationUnauthorizedActor.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-200/ExposureSensitiveInformationUnauthorizedActor.ql

@@ -0,0 +1,65 @@
+/**
+ * @name Writing to a file without setting permissions.
+ * @description Lack of restriction on file access rights can be unsafe.
+ * @kind problem
+ * @id cpp/work-with-file-without-permissions-rights
+ * @problem.severity warning
+ * @precision medium
+ * @tags correctness
+ *       maintainability
+ *       security
+ *       external/cwe/cwe-200
+ *       external/cwe/cwe-264
+ */
+
+import cpp
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
+
+/** Holds for a function `f` that has an argument at index `apos` used to read the file. */
+predicate numberArgumentRead(Function f, int apos) {
+  f.hasGlobalOrStdName("fgets") and apos = 2
+  or
+  f.hasGlobalOrStdName("fread") and apos = 3
+  or
+  f.hasGlobalOrStdName("read") and apos = 0
+  or
+  f.hasGlobalOrStdName("fscanf") and apos = 0
+}
+
+/** Holds for a function `f` that has an argument at index `apos` used to write to file */
+predicate numberArgumentWrite(Function f, int apos) {
+  f.hasGlobalOrStdName("fprintf") and apos = 0
+  or
+  f.hasGlobalOrStdName("fputs") and apos = 1
+  or
+  f.hasGlobalOrStdName("write") and apos = 0
+  or
+  f.hasGlobalOrStdName("fwrite") and apos = 3
+  or
+  f.hasGlobalOrStdName("fflush") and apos = 0
+}
+
+from FunctionCall fc
+where
+  // a file is opened
+  (
+    fc.getTarget().hasGlobalOrStdName("fopen") or
+    fc.getTarget().hasGlobalOrStdName("open")
+  ) and
+  fc.getNumberOfArguments() = 2 and
+  // the file is used for writing (but not reading)
+  exists(FunctionCall fctmp, int i |
+    numberArgumentWrite(fctmp.getTarget(), i) and
+    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
+  ) and
+  not exists(FunctionCall fctmp, int i |
+    numberArgumentRead(fctmp.getTarget(), i) and
+    globalValueNumber(fc) = globalValueNumber(fctmp.getArgument(i))
+  ) and
+  // a file creation mode is not set globally by `umask` anywhere in the program
+  not exists(FunctionCall fctmp |
+    fctmp.getTarget().hasGlobalOrStdName("umask") or
+    fctmp.getTarget().hasGlobalOrStdName("fchmod") or
+    fctmp.getTarget().hasGlobalOrStdName("chmod")
+  )
+select fc, "You may have forgotten to restrict access rights when working with a file."

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/ExposureSensitiveInformationUnauthorizedActor.expected

@@ -0,0 +1 @@
+| test.cpp:12:8:12:12 | call to fopen | You may have forgotten to restrict access rights when working with a file. |

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/ExposureSensitiveInformationUnauthorizedActor.qlref

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test1/test.cpp

@@ -0,0 +1,17 @@
+typedef int FILE;
+FILE *fopen(const char *filename, const char *mode);
+int umask(int pmode);
+int chmod(char * filename,int pmode);
+int fprintf(FILE *fp,const char *fmt, ...);
+int fclose(FILE *stream);
+
+int main(int argc, char *argv[])
+{
+  //umask(0022);
+  FILE *fp;
+  fp = fopen("myFile.txt","w"); // BAD
+  //chmod("myFile.txt",0644);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+  return 0;
+}

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test2/ExposureSensitiveInformationUnauthorizedActor.expected

@@ -0,0 +1 @@
+experimental/Security/CWE/CWE-200/ExposureSensitiveInformationUnauthorizedActor.ql

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-200/test2/ExposureSensitiveInformationUnauthorizedActor.qlref

@@ -0,0 +1,17 @@
+typedef int FILE;
+FILE *fopen(const char *filename, const char *mode);
+int umask(int pmode);
+int chmod(char * filename,int pmode);
+int fprintf(FILE *fp,const char *fmt, ...);
+int fclose(FILE *stream);
+
+int main(int argc, char *argv[])
+{
+  umask(0022);
+  FILE *fp;
+  fp = fopen("myFile.txt","w"); // GOOD
+  chmod("myFile.txt",0644);
+  fprintf(fp,"%s\n","data to file");
+  fclose(fp);
+  return 0;
+}