Merge branch 'main' into swift/extract-mainactor

Author: d10c

cpp/ql/lib/semmle/code/cpp/models/interfaces/Allocation.qll

@@ -12,27 +12,38 @@ import semmle.code.cpp.Function
 import semmle.code.cpp.models.Models
 
 /**
- * An allocation function such as `malloc`.
+ * An allocation expression such as call to `malloc` or a `new` expression.
  */
-abstract class AllocationFunction extends Function {
+abstract class AllocationExpr extends Expr {
   /**
-   * Gets the index of the argument for the allocation size, if any. The actual
-   * allocation size is the value of this argument multiplied by the result of
+   * Gets an expression for the allocation size, if any. The actual allocation
+   * size is the value of this expression multiplied by the result of
    * `getSizeMult()`, in bytes.
    */
-  int getSizeArg() { none() }
+  Expr getSizeExpr() { none() }
 
   /**
-   * Gets the index of an argument that multiplies the allocation size given by
-   * `getSizeArg`, if any.
+   * Gets a constant multiplier for the allocation size given by `getSizeExpr`,
+   * in bytes.
    */
   int getSizeMult() { none() }
 
   /**
-   * Gets the index of the input pointer argument to be reallocated, if this
-   * is a `realloc` function.
+   * Gets the size of this allocation in bytes, if it is a fixed size and that
+   * size can be determined.
    */
-  int getReallocPtrArg() { none() }
+  int getSizeBytes() { none() }
+
+  /**
+   * Gets the expression for the input pointer argument to be reallocated, if
+   * this is a `realloc` function.
+   */
+  Expr getReallocPtr() { none() }
+
+  /**
+   * Gets the type of the elements that are allocated, if it can be determined.
+   */
+  Type getAllocatedElementType() { none() }
 
   /**
    * Whether or not this allocation requires a corresponding deallocation of
@@ -44,38 +55,30 @@ abstract class AllocationFunction extends Function {
 }
 
 /**
- * An allocation expression such as call to `malloc` or a `new` expression.
+ * An allocation function such as `malloc`.
+ *
+ * Note: `AllocationExpr` includes calls to allocation functions, so prefer
+ * to use that class unless you specifically need to reason about functions.
  */
-abstract class AllocationExpr extends Expr {
+abstract class AllocationFunction extends Function {
   /**
-   * Gets an expression for the allocation size, if any. The actual allocation
-   * size is the value of this expression multiplied by the result of
+   * Gets the index of the argument for the allocation size, if any. The actual
+   * allocation size is the value of this argument multiplied by the result of
    * `getSizeMult()`, in bytes.
    */
-  Expr getSizeExpr() { none() }
+  int getSizeArg() { none() }
 
   /**
-   * Gets a constant multiplier for the allocation size given by `getSizeExpr`,
-   * in bytes.
+   * Gets the index of an argument that multiplies the allocation size given by
+   * `getSizeArg`, if any.
    */
   int getSizeMult() { none() }
 
   /**
-   * Gets the size of this allocation in bytes, if it is a fixed size and that
-   * size can be determined.
-   */
-  int getSizeBytes() { none() }
-
-  /**
-   * Gets the expression for the input pointer argument to be reallocated, if
-   * this is a `realloc` function.
-   */
-  Expr getReallocPtr() { none() }
-
-  /**
-   * Gets the type of the elements that are allocated, if it can be determined.
+   * Gets the index of the input pointer argument to be reallocated, if this
+   * is a `realloc` function.
    */
-  Type getAllocatedElementType() { none() }
+  int getReallocPtrArg() { none() }
 
   /**
    * Whether or not this allocation requires a corresponding deallocation of

cpp/ql/lib/semmle/code/cpp/models/interfaces/Deallocation.qll

@@ -12,23 +12,26 @@ import semmle.code.cpp.Function
 import semmle.code.cpp.models.Models
 
 /**
- * A deallocation function such as `free`.
+ * An deallocation expression such as call to `free` or a `delete` expression.
  */
-abstract class DeallocationFunction extends Function {
+abstract class DeallocationExpr extends Expr {
   /**
-   * Gets the index of the argument that is freed by this function.
+   * Gets the expression that is freed by this function.
    */
-  int getFreedArg() { none() }
+  Expr getFreedExpr() { none() }
 }
 
 /**
- * An deallocation expression such as call to `free` or a `delete` expression.
+ * A deallocation function such as `free`.
+ *
+ * Note: `DeallocationExpr` includes calls to deallocation functions, so prefer
+ * to use that class unless you specifically need to reason about functions.
  */
-abstract class DeallocationExpr extends Expr {
+abstract class DeallocationFunction extends Function {
   /**
-   * Gets the expression that is freed by this function.
+   * Gets the index of the argument that is freed by this function.
    */
-  Expr getFreedExpr() { none() }
+  int getFreedArg() { none() }
 }
 
 /**

cpp/ql/src/Critical/MissingCheckScanf.ql

@@ -115,7 +115,8 @@ BasicBlock blockGuardedBy(int value, string op, ScanfFunctionCall call) {
 from ScanfOutput output, ScanfFunctionCall call, Access access
 where
   output.getCall() = call and
-  output.hasGuardedAccess(access, false)
+  output.hasGuardedAccess(access, false) and
+  not exists(DeallocationExpr dealloc | dealloc.getFreedExpr() = access)
 select access,
   "This variable is read, but may not have been written. " +
     "It should be guarded by a check that the $@ returns at least " +

cpp/ql/src/change-notes/2022-12-15-no-scanf-buffer-allocation-warnings.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `cpp/missing-check-scanf` query no longer reports the free'ing of `scanf` output variables as potential reads.

cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected

@@ -1,19 +1,21 @@
-| test.cpp:30:7:30:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:29:3:29:7 | call to scanf | call to scanf |
-| test.cpp:46:7:46:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:45:3:45:7 | call to scanf | call to scanf |
-| test.cpp:63:7:63:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:62:3:62:7 | call to scanf | call to scanf |
-| test.cpp:75:7:75:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:74:3:74:7 | call to scanf | call to scanf |
-| test.cpp:87:7:87:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:86:3:86:8 | call to fscanf | call to fscanf |
-| test.cpp:94:7:94:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:93:3:93:8 | call to sscanf | call to sscanf |
-| test.cpp:143:8:143:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:141:7:141:11 | call to scanf | call to scanf |
-| test.cpp:152:8:152:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:150:7:150:11 | call to scanf | call to scanf |
-| test.cpp:184:8:184:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:183:7:183:11 | call to scanf | call to scanf |
-| test.cpp:203:8:203:8 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:200:7:200:11 | call to scanf | call to scanf |
-| test.cpp:227:9:227:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:225:25:225:29 | call to scanf | call to scanf |
-| test.cpp:231:9:231:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:229:14:229:18 | call to scanf | call to scanf |
-| test.cpp:243:7:243:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:242:3:242:7 | call to scanf | call to scanf |
-| test.cpp:251:7:251:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:250:3:250:7 | call to scanf | call to scanf |
-| test.cpp:259:7:259:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:258:3:258:7 | call to scanf | call to scanf |
-| test.cpp:271:7:271:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:270:3:270:7 | call to scanf | call to scanf |
-| test.cpp:281:8:281:12 | ptr_i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:280:3:280:7 | call to scanf | call to scanf |
-| test.cpp:289:7:289:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:288:3:288:7 | call to scanf | call to scanf |
-| test.cpp:383:25:383:25 | u | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:382:6:382:11 | call to sscanf | call to sscanf |
+| test.cpp:35:7:35:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:34:3:34:7 | call to scanf | call to scanf |
+| test.cpp:51:7:51:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:50:3:50:7 | call to scanf | call to scanf |
+| test.cpp:68:7:68:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:67:3:67:7 | call to scanf | call to scanf |
+| test.cpp:80:7:80:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:79:3:79:7 | call to scanf | call to scanf |
+| test.cpp:90:8:90:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:89:3:89:7 | call to scanf | call to scanf |
+| test.cpp:98:8:98:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:97:3:97:7 | call to scanf | call to scanf |
+| test.cpp:108:7:108:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:107:3:107:8 | call to fscanf | call to fscanf |
+| test.cpp:115:7:115:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:114:3:114:8 | call to sscanf | call to sscanf |
+| test.cpp:164:8:164:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:162:7:162:11 | call to scanf | call to scanf |
+| test.cpp:173:8:173:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:171:7:171:11 | call to scanf | call to scanf |
+| test.cpp:205:8:205:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:204:7:204:11 | call to scanf | call to scanf |
+| test.cpp:224:8:224:8 | j | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:221:7:221:11 | call to scanf | call to scanf |
+| test.cpp:248:9:248:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:246:25:246:29 | call to scanf | call to scanf |
+| test.cpp:252:9:252:9 | d | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 2. | test.cpp:250:14:250:18 | call to scanf | call to scanf |
+| test.cpp:264:7:264:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:263:3:263:7 | call to scanf | call to scanf |
+| test.cpp:272:7:272:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:271:3:271:7 | call to scanf | call to scanf |
+| test.cpp:280:7:280:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:279:3:279:7 | call to scanf | call to scanf |
+| test.cpp:292:7:292:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:291:3:291:7 | call to scanf | call to scanf |
+| test.cpp:302:8:302:12 | ptr_i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:301:3:301:7 | call to scanf | call to scanf |
+| test.cpp:310:7:310:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:309:3:309:7 | call to scanf | call to scanf |
+| test.cpp:404:25:404:25 | u | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:403:6:403:11 | call to sscanf | call to sscanf |

cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp

@@ -19,6 +19,11 @@ FILE *get_a_stream();
 const char *get_a_string();
 extern locale_t get_a_locale();
 
+typedef long size_t;
+
+void *malloc(size_t size);
+void free(void *ptr);
+
 int main()
 {
 	// --- simple cases ---
@@ -78,6 +83,22 @@ int main()
 		use(i); // GOOD
 	}
 
+	{
+		int *i = (int*)malloc(sizeof(int)); // Allocated variable
+
+		scanf("%d", i);
+		use(*i); // BAD
+		free(i); // GOOD
+	}
+
+	{
+		int *i = new int; // Allocated variable
+
+		scanf("%d", i);
+		use(*i); // BAD
+		delete i; // GOOD
+	}
+
 	// --- different scanf functions ---
 
 	{

csharp/ql/lib/ext/Dapper.model.yml

@@ -1,7 +1,7 @@
 extensions:
   - addsTo:
       pack: codeql/csharp-all
-      extensible: extSinkModel
+      extensible: sinkModel
     data:
       - ["Dapper", "SqlMapper", False, "Execute", "(System.Data.IDbConnection,System.String,System.Object,System.Data.IDbTransaction,System.Nullable<System.Int32>,System.Nullable<System.Data.CommandType>)", "", "Argument[1]", "sql", "manual"]
       - ["Dapper", "SqlMapper", False, "ExecuteAsync", "(System.Data.IDbConnection,System.String,System.Object,System.Data.IDbTransaction,System.Nullable<System.Int32>,System.Nullable<System.Data.CommandType>)", "", "Argument[1]", "sql", "manual"]

csharp/ql/lib/ext/Microsoft.ApplicationBlocks.Data.model.yml

@@ -1,7 +1,7 @@
 extensions:
   - addsTo:
       pack: codeql/csharp-all
-      extensible: extSinkModel
+      extensible: sinkModel
     data:
       - ["Microsoft.ApplicationBlocks.Data", "SqlHelper", False, "ExecuteDataset", "(System.Data.SqlClient.SqlConnection,System.Data.CommandType,System.String)", "", "Argument[2]", "sql", "manual"]
       - ["Microsoft.ApplicationBlocks.Data", "SqlHelper", False, "ExecuteDataset", "(System.Data.SqlClient.SqlConnection,System.Data.CommandType,System.String,System.Data.SqlClient.SqlParameter[])", "", "Argument[2]", "sql", "manual"]

csharp/ql/lib/ext/Microsoft.EntityFrameworkCore.model.yml

@@ -1,7 +1,7 @@
 extensions:
   - addsTo:
       pack: codeql/csharp-all
-      extensible: extSinkModel
+      extensible: sinkModel
     data:
       - ["Microsoft.EntityFrameworkCore", "RelationalDatabaseFacadeExtensions", False, "ExecuteSqlRaw", "(Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade,System.String,System.Collections.Generic.IEnumerable<System.Object>)", "", "Argument[1]", "sql", "manual"]
       - ["Microsoft.EntityFrameworkCore", "RelationalDatabaseFacadeExtensions", False, "ExecuteSqlRaw", "(Microsoft.EntityFrameworkCore.Infrastructure.DatabaseFacade,System.String,System.Object[])", "", "Argument[1]", "sql", "manual"]

csharp/ql/lib/ext/Microsoft.Extensions.Primitives.model.yml

@@ -1,7 +1,7 @@
 extensions:
   - addsTo:
       pack: codeql/csharp-all
-      extensible: extSummaryModel
+      extensible: summaryModel
     data:
       - ["Microsoft.Extensions.Primitives", "StringValues", False, "Add", "(System.String)", "", "Argument[0]", "ReturnValue", "taint", "manual"]
       - ["Microsoft.Extensions.Primitives", "StringValues", False, "Add", "(System.String)", "", "Argument[this]", "ReturnValue", "taint", "manual"]