fix warnings, more sinks,sources,comments

Author: am0o0

cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/DecompressionBombsGzopen.ql

@@ -16,50 +16,65 @@ import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.security.FlowSources
 
 /**
- * A gzFile Variable as a Flow source
+ * A `gzFile` Variable as a Flow source
  */
 private class GzFileVar extends VariableAccess {
   GzFileVar() { this.getType().hasName("gzFile") }
 }
 
 /**
  * The `gzopen` function as a Flow source
+ *
+ * `gzopen(const char *path, const char *mode)`
  */
 private class GzopenFunction extends Function {
-  GzopenFunction() { hasGlobalName("gzopen") }
+  GzopenFunction() { this.hasGlobalName("gzopen") }
 }
 
 /**
  * The `gzdopen` function as a Flow source
+ *
+ * `gzdopen(int fd, const char *mode)`
  */
 private class GzdopenFunction extends Function {
-  GzdopenFunction() { hasGlobalName("gzdopen") }
+  GzdopenFunction() { this.hasGlobalName("gzdopen") }
 }
 
 /**
  * The `gzfread` function is used in Flow sink
+ *
+ * `gzfread(voidp buf, z_size_t size, z_size_t nitems, gzFile file)`
  */
 private class GzfreadFunction extends Function {
-  GzfreadFunction() { hasGlobalName("gzfread") }
+  GzfreadFunction() { this.hasGlobalName("gzfread") }
+}
+
+/**
+ * The `gzgets` function is used in Flow sink.
+ *
+ * `gzgets(gzFile file, char *buf, int len)`
+ */
+private class GzgetsFunction extends Function {
+  GzgetsFunction() { this.hasGlobalName("gzgets") }
 }
 
 /**
  * The `gzread` function is used in Flow sink
+ *
+ * `gzread(gzFile file, voidp buf, unsigned len)`
  */
 private class GzreadFunction extends Function {
-  GzreadFunction() { hasGlobalName("gzread") }
+  GzreadFunction() { this.hasGlobalName("gzread") }
 }
 
 module ZlibTaintConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
-    // gzopen(const char *path, const char *mode);
     exists(FunctionCall fc | fc.getTarget() instanceof GzopenFunction |
       fc.getArgument(0) = source.asExpr() and
       // arg 0 can be a path string whichwe must do following check
       not fc.getArgument(0).isConstant()
     )
     or
-    //gzdopen(int fd, const char *mode);
     // IDK whether it is good to use all file decriptors function returns as source or not
     // because we can do more sanitization from fd function sources
     exists(FunctionCall fc | fc.getTarget() instanceof GzdopenFunction |
@@ -70,40 +85,43 @@ module ZlibTaintConfig implements DataFlow::ConfigSig {
   }
 
   predicate isSink(DataFlow::Node sink) {
-    // gzread(gzFile file, voidp buf, unsigned len));
     exists(FunctionCall fc | fc.getTarget() instanceof GzreadFunction |
       fc.getArgument(0) = sink.asExpr() and
       not sanitizer(fc)
       // TODO: and not sanitizer2(fc.getArgument(2))
     )
     or
-    // gzfread((voidp buf, z_size_t size, z_size_t nitems, gzFile file));
     exists(FunctionCall fc | fc.getTarget() instanceof GzfreadFunction |
       sink.asExpr() = fc.getArgument(3)
     )
+    or
+    exists(FunctionCall fc | fc.getTarget() instanceof GzgetsFunction |
+      sink.asExpr() = fc.getArgument(0)
+    )
   }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    // gzopen(const char *path, const char *mode);
-    // gzdopen(int fd, const char *mode);
     exists(FunctionCall fc |
       fc.getTarget() instanceof GzopenFunction or fc.getTarget() instanceof GzdopenFunction
     |
       node1.asExpr() = fc.getArgument(0) and
       node2.asExpr() = fc
     )
     or
-    // gzread(gzFile file, voidp buf, unsigned len);
     exists(FunctionCall fc | fc.getTarget() instanceof GzreadFunction |
       node1.asExpr() = fc.getArgument(0) and
       node2.asExpr() = fc.getArgument(1)
     )
     or
-    // gzfread(voidp buf, z_size_t size, z_size_t nitems, gzFile file);
     exists(FunctionCall fc | fc.getTarget() instanceof GzfreadFunction |
       node1.asExpr() = fc.getArgument(3) and
       node2.asExpr() = fc.getArgument(0)
     )
+    or
+    exists(FunctionCall fc | fc.getTarget() instanceof GzgetsFunction |
+      node1.asExpr() = fc.getArgument(0) and
+      node1.asExpr() = fc.getArgument(1)
+    )
   }
 }
 
@@ -116,19 +134,12 @@ predicate sanitizer(FunctionCall fc) {
 }
 
 // TODO:
-// predicate sanitizer2(Expr arg) {
+// predicate sanitizer2(FunctionCall fc) {
 //   exists(Expr e |
 //     // a RelationalOperation which isn't compared with a Literal that using for end of read
-//     TaintTracking::localExprTaint(arg, e.(RelationalOperation).getAChild*())
+//     TaintTracking::localExprTaint(fc.getArgument(2), e)
 //   )
 // }
-// predicate test(FunctionCall fc, Expr sink) {
-//   exists( | fc.getTarget() instanceof GzreadFunction |
-//     // a RelationalOperation which isn't compared with a Literal that using for end of read
-//     TaintTracking::localExprTaint(fc.getArgument(2).(VariableAccess), sink)
-//   ) and
-//   sink.getFile().getLocation().toString().matches("%main.cpp%")
-// }
 module ZlibTaint = TaintTracking::Global<ZlibTaintConfig>;
 
 import ZlibTaint::PathGraph

cpp/ql/src/experimental/Security/CWE/CWE-409-DecompressionBomb/DecompressionBombsInflator.ql

@@ -16,30 +16,34 @@ import semmle.code.cpp.ir.dataflow.TaintTracking
 import semmle.code.cpp.security.FlowSources
 
 /**
- * A z_stream Variable as a Flow source
+ * A `z_stream` Variable as a Flow source
  */
 private class ZStreamVar extends VariableAccess {
   ZStreamVar() { this.getType().hasName("z_stream") }
 }
 
 /**
- * The `inflate`/`inflateSync` function is used in Flow sink
+ * The `inflate`/`inflateSync` functions are used in Flow sink
+ *
+ * `inflate(z_streamp strm, int flush)`
+ *
+ * `inflateSync(z_streamp strm)`
  */
-private class DeflateFunction extends Function {
-  DeflateFunction() { hasGlobalName(["inflate", "inflateSync"]) }
+private class InflateFunction extends Function {
+  InflateFunction() { this.hasGlobalName(["inflate", "inflateSync"]) }
 }
 
 module ZlibTaintConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) { source.asExpr() instanceof ZStreamVar }
 
   predicate isSink(DataFlow::Node sink) {
-    exists(FunctionCall fc | fc.getTarget() instanceof DeflateFunction |
+    exists(FunctionCall fc | fc.getTarget() instanceof InflateFunction |
       fc.getArgument(0) = sink.asExpr()
     )
   }
 
   predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FunctionCall fc | fc.getTarget() instanceof DeflateFunction |
+    exists(FunctionCall fc | fc.getTarget() instanceof InflateFunction |
       node1.asExpr() = fc.getArgument(0) and
       node2.asExpr() = fc
     )