Merge pull request #2300 from geoffw0/names

CPP: Rename/relocate some queries and libraries

cpp/ql/src/Likely Bugs/Memory Management/Buffer Overflow/NtohlArrayNoBound.qll

@@ -1,33 +0,0 @@
-import cpp
-import semmle.code.cpp.dataflow.DataFlow
-import semmle.code.cpp.controlflow.Guards
-import BufferAccess
-import semmle.code.cpp.valuenumbering.GlobalValueNumbering
-
-class NetworkFunctionCall extends FunctionCall {
-  NetworkFunctionCall() {
-    getTarget().hasName("ntohd") or
-    getTarget().hasName("ntohf") or
-    getTarget().hasName("ntohl") or
-    getTarget().hasName("ntohll") or
-    getTarget().hasName("ntohs")
-  }
-}
-
-class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {
-  NetworkToBufferSizeConfiguration() { this = "NetworkToBufferSizeConfiguration" }
-
-  override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof NetworkFunctionCall }
-
-  override predicate isSink(DataFlow::Node node) {
-    node.asExpr() = any(BufferAccess ba).getAccessedLength()
-  }
-
-  override predicate isBarrier(DataFlow::Node node) {
-    exists(GuardCondition gc, GVN gvn |
-      gc.getAChild*() = gvn.getAnExpr() and
-      globalValueNumber(node.asExpr()) = gvn and
-      gc.controls(node.asExpr().getBasicBlock(), _)
-    )
-  }
-}

cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.qhelp

@@ -31,10 +31,10 @@ it to host byte order. The data is then used as an index in an array access expr
 there is no validation that the data returned by <code>ntohl</code> is within the bounds of the array,
 which could lead to reading outside the bounds of the buffer.
 </p>
-<sample src="NtohlArrayBad.cpp" />
+<sample src="NtohlArrayNoBound-bad.cpp" />
 <p>In the corrected example, the returned data is validated against the known size of the buffer,
 before being used as an array index.</p>
-<sample src="NtohlArrayGood.cpp" />
+<sample src="NtohlArrayNoBound-good.cpp" />
 </example>
 
 <references>

cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.qll

@@ -1,55 +1,7 @@
 import cpp
-import semmle.code.cpp.dataflow.TaintTracking
-private import semmle.code.cpp.dataflow.RecursionPrevention
-
-/**
- * A buffer which includes an allocation size.
- */
-abstract class BufferWithSize extends DataFlow::Node {
-  abstract Expr getSizeExpr();
-
-  BufferAccess getAnAccess() {
-    any(BufferWithSizeConfig bsc).hasFlow(this, DataFlow::exprNode(result.getPointer()))
-  }
-}
-
-/** An allocation function. */
-abstract class Alloc extends Function { }
-
-/**
- * Allocation functions identified by the QL for C/C++ standard library.
- */
-class DefaultAlloc extends Alloc {
-  DefaultAlloc() { allocationFunction(this) }
-}
-
-/** A buffer created through a call to an allocation function. */
-class AllocBuffer extends BufferWithSize {
-  FunctionCall call;
-
-  AllocBuffer() {
-    asExpr() = call and
-    call.getTarget() instanceof Alloc
-  }
-
-  override Expr getSizeExpr() { result = call.getArgument(0) }
-}
-
-/**
- * Find accesses of buffers for which we have a size expression.
- */
-private class BufferWithSizeConfig extends TaintTracking::Configuration {
-  BufferWithSizeConfig() { this = "BufferWithSize" }
-
-  override predicate isSource(DataFlow::Node n) { n = any(BufferWithSize b) }
-
-  override predicate isSink(DataFlow::Node n) { n.asExpr() = any(BufferAccess ae).getPointer() }
-
-  override predicate isSanitizer(DataFlow::Node s) {
-    s = any(BufferWithSize b) and
-    s.asExpr().getControlFlowScope() instanceof Alloc
-  }
-}
+import semmle.code.cpp.dataflow.DataFlow
+import semmle.code.cpp.controlflow.Guards
+import semmle.code.cpp.valuenumbering.GlobalValueNumbering
 
 /**
  * An access (read or write) to a buffer, provided as a pair of
@@ -172,3 +124,31 @@ class MallocSizeExpr extends BufferAccess, FunctionCall {
 
   override Expr getAccessedLength() { result = getArgument(1) }
 }
+
+class NetworkFunctionCall extends FunctionCall {
+  NetworkFunctionCall() {
+    getTarget().hasName("ntohd") or
+    getTarget().hasName("ntohf") or
+    getTarget().hasName("ntohl") or
+    getTarget().hasName("ntohll") or
+    getTarget().hasName("ntohs")
+  }
+}
+
+class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {
+  NetworkToBufferSizeConfiguration() { this = "NetworkToBufferSizeConfiguration" }
+
+  override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof NetworkFunctionCall }
+
+  override predicate isSink(DataFlow::Node node) {
+    node.asExpr() = any(BufferAccess ba).getAccessedLength()
+  }
+
+  override predicate isBarrier(DataFlow::Node node) {
+    exists(GuardCondition gc, GVN gvn |
+      gc.getAChild*() = gvn.getAnExpr() and
+      globalValueNumber(node.asExpr()) = gvn and
+      gc.controls(node.asExpr().getBasicBlock(), _)
+    )
+  }
+}

cpp/ql/src/Microsoft/IgnoreReturnValueSAL.ql

@@ -7,6 +7,10 @@
  * @id cpp/ignore-return-value-sal
  * @problem.severity warning
  * @tags reliability
+ *       external/cwe/cwe-573
+ *       external/cwe/cwe-252
+ * @opaque-id SM02344
+ * @microsoft.severity Important
  */
 
 import SAL

cpp/ql/src/Microsoft/SAL/IgnoreReturnValueSAL.ql

@@ -1,25 +0,0 @@
-/**
- * @name SAL requires inspecting return value
- * @description When a return value is discarded even though the SAL annotation
- *              requires inspecting it, a recoverable error may turn into a
- *              whole-program crash.
- * @kind problem
- * @problem.severity warning
- * @tags reliability
- *       external/cwe/cwe-573
- *       external/cwe/cwe-252
- * @opaque-id SM02344
- * @microsoft.severity Important
- * @id cpp/ignorereturnvaluesal
- */
-
-import Microsoft.SAL
-
-from Function f, FunctionCall call
-where
-  call.getTarget() = f and
-  call instanceof ExprInVoidContext and
-  any(SALCheckReturn a).getDeclaration() = f and
-  not any(Options o).okToIgnoreReturnValue(call)
-select call, "Return value of $@ discarded although a SAL annotation " + "requires inspecting it.",
-  f, f.getName()

cpp/ql/src/Security/CWE/CWE-457/InitializationFunctions.qll

@@ -4,7 +4,7 @@
 
 import cpp
 import external.ExternalArtifact
-private import semmle.code.cpp.dispatch.VirtualDispatch
+private import semmle.code.cpp.dispatch.VirtualDispatchPrototype
 import semmle.code.cpp.NestedFields
 import Microsoft.SAL
 import semmle.code.cpp.controlflow.Guards

cpp/ql/test/query-tests/Likely Bugs/Memory Management/Buffer Overflow/NtohlArrayNoBound/NtohlArrayNoBound.qlref

@@ -1 +0,0 @@
-Likely Bugs/Memory Management/Buffer Overflow/NtohlArrayNoBoundOpenSource.ql

cpp/ql/test/query-tests/Likely Bugs/Memory Management/NtohlArrayNoBound/NtohlArrayNoBound.qlref

@@ -0,0 +1 @@
+Likely Bugs/Memory Management/NtohlArrayNoBound.ql

cpp/ql/test/query-tests/Likely Bugs/Protocols/TlsSettingsMisconfiguration.qlref

@@ -0,0 +1 @@
+Likely Bugs/Protocols/TlsSettingsMisconfiguration.ql

cpp/ql/test/query-tests/Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.qlref

@@ -0,0 +1 @@
+Likely Bugs/Protocols/UseOfDeprecatedHardcodedProtocol.ql

cpp/ql/test/query-tests/Likely Bugs/Protocols/boostorg/TlsSettingsMisconfiguration.qlref

@@ -1 +0,0 @@
-Likely Bugs/Protocols/boostorg/TlsSettingsMisconfiguration.ql

cpp/ql/test/query-tests/Likely Bugs/Protocols/boostorg/UseOfDeprecatedHardcodedProtocol.qlref

@@ -1 +0,0 @@
-Likely Bugs/Protocols/boostorg/UseOfDeprecatedHardcodedProtocol.ql