codeql chores

update queries, delte obsolte queries, fix findings, add back simpler version of alloca in loop

Author: two-heart

contrib/codeql/dev/VisualizeTopology.ql

@@ -1,6 +1,6 @@
 import cpp
 
-/* Point this predicate to the topology to be visualized. Make sure it is part of the DB */
+  /* Point this predicate to the topology to be visualized */
 predicate inTopology(Location loc) {
   loc.getFile().getRelativePath() = "src/app/firedancer/topology.c" or
   loc.getFile().getRelativePath() = "src/disco/net/fd_net_tile_topo.c"
@@ -70,7 +70,7 @@ class OutLink extends FunctionCall {
     or
     (
     this.getTarget().hasName("fd_topos_net_rx_link") and
-    name = this.getArgument(2).(StringLiteral).getValue() and
+    name = this.getArgument(1).(StringLiteral).getValue() and
     out_tile = "net" and /* leaving aside the sock alternative */
     inTopology(this.getLocation())
     )

contrib/codeql/nightly/AccountDerefBeforeModify.ql

@@ -0,0 +1,24 @@
+/**
+ * @name Alloca call in loop
+ * @description We want to strictly forbid alloca in loops to avoid
+ * stack overflows
+ * @kind problem
+ * @problem.severity warning
+ * @precision high
+ * @id asymmetric-research/alloca-in-loop
+ */
+
+import cpp
+
+class AllocaCall extends FunctionCall {
+  AllocaCall() {
+    this.getTarget().hasName("fd_alloca") or
+    this.getTarget().hasName("alloca") or
+    this.getTarget().hasName("fd_alloca_check") or
+    this.getTarget().hasName("__builtin_alloca")
+  }
+}
+
+from Loop l, AllocaCall c
+where c.getAPredecessor*() = l
+select c, "Call to alloca in loop"

contrib/codeql/nightly/AllocaCallInLoop.ql

@@ -23,12 +23,27 @@ class MemcpyFunction extends Function {
   }
 }
 
-from FunctionCall call, MemcpyFunction memcpy
+predicate ignoredLocation(Location l) {
+  // we don't want to change vendored code if not really necessary
+  l.getFile().getBaseName() = "cJSON.c"
+}
+
+class InScopeType extends Type {
+  InScopeType() {
+    not this instanceof CharType and
+    not this instanceof VoidType and
+    not this.getUnspecifiedType().(DerivedType).getBaseType().getUnspecifiedType().hasName(["fd_txn_p", "fd_hash"])
+  }
+}
+
+from FunctionCall call, MemcpyFunction memcpy, InScopeType t
 where
   included(call.getLocation()) and
   not call.isInMacroExpansion() and
+  not ignoredLocation(call.getLocation()) and
   call.getTarget() = memcpy and
   call.getArgument(2) instanceof SizeofTypeOperator and
-  call.getArgument(0).getUnspecifiedType() = call.getArgument(1).getUnspecifiedType() and
-  call.getArgument(0).getUnspecifiedType().(DerivedType).getBaseType().getUnspecifiedType() = call.getArgument(2).(SizeofTypeOperator).getTypeOperand().getUnspecifiedType()
-select call, "Call to " + memcpy.getName() + " could be rewritten as an assignment."
+  t = call.getArgument(0).getUnspecifiedType() and
+  t = call.getArgument(1).getUnspecifiedType() and
+  t.(DerivedType).getBaseType().getUnspecifiedType() = call.getArgument(2).(SizeofTypeOperator).getTypeOperand().getUnspecifiedType()
+select call, "Call to " + memcpy.getName() + " could be rewritten as an assignment." + t.getUnderlyingType()

contrib/codeql/nightly/TrivialMemcpy.ql

@@ -114,7 +114,6 @@ funk_loose( fd_topo_t const *     topo,
 static void
 funk_new( fd_topo_t const *     topo,
            fd_topo_obj_t const * obj ) {
-  (void)topo;
   ulong funk_seed = fd_pod_queryf_ulong( topo->props, 0UL, "obj.%lu.seed", obj->id );
   if( !funk_seed ) FD_TEST( fd_rng_secure( &funk_seed, sizeof(ulong) ) );
   FD_TEST( fd_funk_new( fd_topo_obj_laddr( topo, obj->id ), 2UL, funk_seed, VAL("txn_max"), VAL("rec_max") ) );

src/app/firedancer/callbacks.c

@@ -286,9 +286,6 @@ crypto_realloc( void *       addr,
                 ulong        num,
                 char const * file,
                 int          line ) {
-  (void)file;
-  (void)line;
-
   if( FD_UNLIKELY( !addr ) ) return crypto_malloc( num, file, line );
   if( FD_UNLIKELY( !num ) ) {
     crypto_free( addr, file, line );

src/disco/bundle/fd_bundle_tile.c

@@ -160,10 +160,10 @@ static inline void
 during_frag( fd_capture_tile_ctx_t * ctx,
              ulong                   in_idx,
              ulong                   seq     FD_PARAM_UNUSED,
-             ulong                   sig     FD_PARAM_UNUSED,
+             ulong                   sig,
              ulong                   chunk,
              ulong                   sz,
-             ulong                   ctl FD_PARAM_UNUSED ) {
+             ulong                   ctl ) {
   ctx->skip_frag = 0;
   if( ctx->in_kind[ in_idx ]==SHRED_REPAIR ) {
     if( !is_fec_completes_msg( sz ) ) {
@@ -343,7 +343,7 @@ after_frag( fd_capture_tile_ctx_t * ctx,
 
 static ulong
 populate_allowed_fds( fd_topo_t const      * topo        FD_PARAM_UNUSED,
-                      fd_topo_tile_t const * tile        FD_PARAM_UNUSED,
+                      fd_topo_tile_t const * tile,
                       ulong                  out_fds_cnt FD_PARAM_UNUSED,
                       int *                  out_fds ) {
   ulong out_cnt = 0UL;

src/discof/shredcap/fd_shredcap_tile.c

@@ -263,12 +263,12 @@ during_frag( ctx_t * ctx,
 static void
 after_frag( ctx_t *             ctx,
             ulong               in_idx,
-            ulong               seq FD_PARAM_UNUSED,
+            ulong               seq     FD_PARAM_UNUSED,
             ulong               sig,
             ulong               sz,
-            ulong               tsorig FD_PARAM_UNUSED,
-            ulong               tspub FD_PARAM_UNUSED,
-            fd_stem_context_t * stem FD_PARAM_UNUSED ) {
+            ulong               tsorig,
+            ulong               tspub   FD_PARAM_UNUSED,
+            fd_stem_context_t * stem ) {
   uint in_kind = ctx->in_kind[in_idx];
   if( FD_UNLIKELY( in_kind != IN_KIND_REPLAY ) ) return;
 

src/discof/tower/fd_tower_tile.c

@@ -230,7 +230,7 @@ fd_exec_slot_ctx_recover( fd_exec_slot_ctx_t *                slot_ctx,
   /* Fee Rate Governor */
 
   fd_fee_rate_governor_t * fee_rate_governor = fd_bank_fee_rate_governor_modify( slot_ctx->bank );
-  fd_memcpy( fee_rate_governor, &old_bank->fee_rate_governor, sizeof(fd_fee_rate_governor_t) );
+  *fee_rate_governor = old_bank->fee_rate_governor;
 
   /* Capitalization */