fix query

Author: GrosQuildu

cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.c

@@ -3,100 +3,17 @@
 #include <stdlib.h>
 #include <string.h>
 
-size_t min(size_t a, size_t b) {
-	return a < b ? a : b;
-}
-
-void MlasReorderInputNhwc(
-    const float* S,
-    float* D,
-    size_t InputChannels,
-    size_t RowCount,
-    size_t FullRowCount
-    )
-{
-    const size_t BlockSize = FullRowCount % 123;
-
-    //
-    // Iterate over batches of the input size to improve locality.
-    //
-
-    for (size_t OuterRowCountRemaining = RowCount; OuterRowCountRemaining > 0; ) {
-
-        size_t OuterRowCountBatch = 32;
-
-        const size_t OuterRowCountThisIteration = min(OuterRowCountRemaining, OuterRowCountBatch);
-        OuterRowCountRemaining -= OuterRowCountThisIteration;
-
-        //
-        // Iterate over BlockSize batches of the input channels.
-        //
-
-        const float* s = S;
-        float* d = D;
-
-        for (size_t i = InputChannels; i > 0;) {
-
-            const size_t InputChannelsThisIteration = min(i, BlockSize);
-            i -= InputChannelsThisIteration;
-
-            const float* ss = s;
-            float* dd = d;
-            size_t InnerRowCountRemaining = OuterRowCountThisIteration;
-
-            if (InputChannelsThisIteration == BlockSize) {
-
-                if (BlockSize == 8) {
-
-                    while (InnerRowCountRemaining-- > 0) {
-                        ss += InputChannels;
-                        dd += 8;
-                    }
-
-                } else {
-
-                    while (InnerRowCountRemaining-- > 0) {
-                        ss += InputChannels;
-                        dd += 16;
-                    }
-                }
-
-            } else {
-
-                size_t BlockPadding = BlockSize - InputChannelsThisIteration;
-
-                while (InnerRowCountRemaining-- > 0) {
-                    ss += InputChannels;
-                    dd += BlockSize;
-                }
-            }
-
-            s += InputChannelsThisIteration;
-            d += BlockSize * FullRowCount;
-        }
-
-        S += InputChannels * OuterRowCountThisIteration;
-        D += BlockSize * OuterRowCountThisIteration;
-    }
-}
-
 // from https://github.com/apple-oss-distributions/Libinfo/blob/9fce29e5c5edc15d3ecea55116ca17d3f6350603/lookup.subproj/mdns_module.c#L1033C1-L1079C2
 char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
 {
 	int i = 0, j = 0;
-	// uint32_t len;
 	uint32_t domainlen = 0;
 	char *domain = NULL;
 
 	if ((data == NULL) || (datalen == 0)) return NULL;
 
-	/*
-	 * i: index into input data
-	 * j: index into output string
-	 */
 	while (datalen-- > 0)
 	{
-		// printf("%d\n", len);
 		uint32_t len = data[i++];
 		domainlen += (len + 1);
 		domain = reallocf(domain, domainlen);
@@ -136,4 +53,3 @@ int main() {
     memcpy(data, "\x04quildu\x03xyz\x00", 11);
     _mdns_parse_domain_name(data, datalen);
 }
-

cpp/src/security/DecOverflowWhenComparing/DecOverflowWhenComparing.ql

@@ -2,7 +2,7 @@
  * @name Decrementation overflow when comparing
  * @id tob/cpp/dec-overflow-when-comparing
  * @description This query finds unsigned integer overflows resulting from unchecked decrementation during comparison.
- * @kind graph
+ * @kind problem
  * @tags security
  * @problem.severity error
  * @precision high
@@ -14,49 +14,24 @@ import cpp
 import semmle.code.cpp.ir.IR
 import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
 
-query predicate nodes(ControlFlowNode node, string key, string value) {
-  exists(Variable var, PostfixDecrExpr dec |
-    dec.getOperand() = var.getAnAccess().getExplicitlyConverted() and
-    var.getUnderlyingType().(IntegralType).isUnsigned() and
-    successorGuarded(node, _, var) and
-    key = node.toString() and
-    value = node.toString() + "-val"
-  )
-}
-
-query predicate edges(ControlFlowNode source, ControlFlowNode target, string key, string value) {
-  exists(Variable var, PostfixDecrExpr dec, VariableAccess acc |
-    var.getAnAccess() = acc and
-    dec.getOperand() = acc.getExplicitlyConverted() and
-    var.getUnderlyingType().(IntegralType).isUnsigned() and
-    
-    source.getASuccessor() = target and
-
-    key = source.toString()  + "-key" and
-    value = target.toString() + "-val"
-  )
-}
-
-query predicate graphProperties(string key, string value) {
-  key = "semmle.graphKind" and value = "graph"
+/**
+ * Holds if `node` overwrites `var` (assignment or declaration with initializer).
+ */
+predicate isDefOf(ControlFlowNode node, Variable var) {
+  node = var.getAnAccess() and node.(VariableAccess).isLValue()
+  or
+  node.(DeclStmt).getADeclaration() = var and exists(var.getInitializer())
 }
 
 /**
- * Find CFG paths from start to end that do not cross over node that is var's lvalue access
- * TODO: there must be an API for that...
+ * Find CFG paths from start to end that do not cross over a definition of var.
  */
 predicate successorGuarded(ControlFlowNode start, ControlFlowNode end, Variable var) {
   start = end
   or
   exists(ControlFlowNode interm |
     start.getASuccessor() = interm and
-
-    // break the path if variable is overwritten
-    not (
-      interm = var.getAnAccess() and
-      interm.(VariableAccess).isLValue()
-    ) and
-
+    not isDefOf(interm, var) and
     (
       interm.getASuccessor() = end
       or
@@ -65,7 +40,7 @@ predicate successorGuarded(ControlFlowNode start, ControlFlowNode end, Variable
   )
 }
 
-/*
+
 from Variable var, VariableAccess varAcc, PostfixDecrExpr dec,
   VariableAccess varAccAfterOverflow, ComparisonOperation cmp
 where
@@ -103,9 +78,7 @@ where
   // only if var may possibly be zero during comparison
   lowerBound(varAcc) = 0
 
-  // skip tests etc
-  and not dec.getFile().getAbsolutePath().toLowerCase().matches(["%test%", "%vendor%", "%third_party%"])
+  // skip vendor code
+  and not dec.getFile().getAbsolutePath().toLowerCase().matches(["%vendor%", "%third_party%"])
 
 select dec, "Unsigned decrementation in comparison ($@) - $@", cmp, cmp.toString(), varAccAfterOverflow, varAccAfterOverflow.toString()
-
-*/

cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.c

@@ -7,19 +7,14 @@
 char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
 {
 	int i = 0, j = 0;
-	uint32_t len;
 	uint32_t domainlen = 0;
 	char *domain = NULL;
 
 	if ((data == NULL) || (datalen == 0)) return NULL;
 
-	/*
-	 * i: index into input data
-	 * j: index into output string
-	 */
 	while (datalen-- > 0)
 	{
-		len = data[i++];
+		uint32_t len = data[i++];
 		domainlen += (len + 1);
 		domain = reallocf(domain, domainlen);
 
@@ -53,7 +48,8 @@ char* _mdns_parse_domain_name(const uint8_t *data, uint32_t datalen)
 }
 
 int main() {
-    uint8_t data[128] = {0};
+    const uint16_t datalen = 128;
+    uint8_t data[datalen] = {};
     memcpy(data, "\x04quildu\x03xyz\x00", 11);
-    _mdns_parse_domain_name(data, 128);
+    _mdns_parse_domain_name(data, datalen);
 }

cpp/test/query-tests/security/DecOverflowWhenComparing/DecOverflowWhenComparing.expected

@@ -0,0 +1 @@
+| DecOverflowWhenComparing.c:30:31:30:39 | ... -- | Unsigned decrementation in comparison ($@) - $@ | DecOverflowWhenComparing.c:30:26:30:39 | ... != ... | ... != ... | DecOverflowWhenComparing.c:15:9:15:15 | datalen | datalen |