Fix use-after-free on bytecode load/execution.

lsig->bc was referring to the bytecode directly (via a pointer),
but the bytecode struct changes place in memory (it is realloced on each .cbc
load). So use an index instead of the direct pointer.

libclamav/bytecode.c

@@ -1636,12 +1636,14 @@ int cli_bytecode_context_setfile(struct cli_bc_ctx *ctx, fmap_t *map)
     return 0;
 }
 
-int cli_bytecode_runlsig(cli_ctx *cctx, const struct cli_all_bc *bcs, const struct cli_bc *bc, const char **virname, const uint32_t* lsigcnt, const uint32_t *lsigsuboff, fmap_t *map)
+int cli_bytecode_runlsig(cli_ctx *cctx, const struct cli_all_bc *bcs, unsigned bc_idx, const char **virname, const uint32_t* lsigcnt, const uint32_t *lsigsuboff, fmap_t *map)
 {
     int ret;
     struct cli_bc_ctx ctx;
+    const struct cli_bc *bc = &bcs->all_bcs[bc_idx-1];
 
     if (bc->hook_lsig_id) {
+	cli_dbgmsg("hook lsig id %d matched (bc %d)\n", bc->hook_lsig_id, bc->id);
 	/* this is a bytecode for a hook, defer running it until hook is
 	 * executed, so that it has all the info for the hook */
 	if (cctx->hook_lsig_matches)
@@ -1687,7 +1689,7 @@ int cli_bytecode_runhook(cli_ctx *cctx, const struct cl_engine *engine, struct c
 	    if (!cctx->hook_lsig_matches ||
 		!cli_bitset_test(cctx->hook_lsig_matches, bc->hook_lsig_id-1))
 		continue;
-	    cli_dbgmsg("Bytecode: executing bytecode %u (lsig matched)" , bc->id);
+	    cli_dbgmsg("Bytecode: executing bytecode %u (lsig matched)\n" , bc->id);
 	}
 	cli_bytecode_context_setfuncid(ctx, bc, 0);
 	ret = cli_bytecode_run(&engine->bcs, bc, ctx);

libclamav/bytecode.h

@@ -108,7 +108,7 @@ void cli_bytecode_describe(const struct cli_bc *bc);
 /* Hooks */
 struct cli_exe_info;
 struct cli_ctx_tag;
-int cli_bytecode_runlsig(struct cli_ctx_tag *ctx, const struct cli_all_bc *bcs, const struct cli_bc *bc, const char **virname, const uint32_t* lsigcnt, const uint32_t *lsigsuboff, fmap_t *map);
+int cli_bytecode_runlsig(struct cli_ctx_tag *ctx, const struct cli_all_bc *bcs, unsigned bc_idx, const char **virname, const uint32_t* lsigcnt, const uint32_t *lsigsuboff, fmap_t *map);
 int cli_bytecode_runhook(struct cli_ctx_tag *cctx, const struct cl_engine *engine, struct cli_bc_ctx *ctx, unsigned id, fmap_t *map, const char **virname);
 
 #ifdef __cplusplus

libclamav/matcher.c

@@ -506,13 +506,13 @@ int cli_fmap_scandesc(cli_ctx *ctx, cli_file_t ftype, uint8_t ftonly, struct cli
 		    continue; \
 		} \
 	    } \
-	    if(!xroot->ac_lsigtable[i]->bc) { \
+	    if(!xroot->ac_lsigtable[i]->bc_idx) { \
 		if(ctx->virname) \
 		    *ctx->virname = xroot->ac_lsigtable[i]->virname; \
 		ret = CL_VIRUS; \
 		break; \
 	    } \
-	    if(cli_bytecode_runlsig(ctx, &ctx->engine->bcs, xroot->ac_lsigtable[i]->bc, ctx->virname, xdata.lsigcnt[i], xdata.lsigsuboff[i], map) == CL_VIRUS) { \
+	    if(cli_bytecode_runlsig(ctx, &ctx->engine->bcs, xroot->ac_lsigtable[i]->bc_idx, ctx->virname, xdata.lsigcnt[i], xdata.lsigsuboff[i], map) == CL_VIRUS) { \
 		ret = CL_VIRUS; \
 		break; \
 	    } \

libclamav/matcher.h

@@ -72,7 +72,7 @@ struct cli_ac_lsig {
     char *logic;
     const char *virname;
     struct cli_lsig_tdb tdb;
-    const struct cli_bc *bc;
+    unsigned bc_idx;
 };
 
 struct cli_matcher {

libclamav/readdb.c

@@ -1115,7 +1115,7 @@ static int lsigattribs(char *attribs, struct cli_lsig_tdb *tdb)
   } while(0);
 
 #define LDB_TOKENS 67
-static int load_oneldb(char *buffer, int chkpua, int chkign, struct cl_engine *engine, unsigned int options, const char *dbname, unsigned int line, unsigned int *sigs, struct cli_bc *bc, const char *buffer_cpy)
+static int load_oneldb(char *buffer, int chkpua, int chkign, struct cl_engine *engine, unsigned int options, const char *dbname, unsigned int line, unsigned int *sigs, unsigned bc_idx, const char *buffer_cpy)
 {
     const char *sig, *virname, *offset, *logic;
     struct cli_ac_lsig **newtable, *lsig;
@@ -1230,7 +1230,9 @@ static int load_oneldb(char *buffer, int chkpua, int chkign, struct cl_engine *e
 	mpool_free(engine->mempool, lsig);
 	return CL_EMEM;
     }
-    lsig->bc = bc;
+    /* 0 marks no bc, we can't use a pointer to bc, since that is
+     * realloced/moved during load */
+    lsig->bc_idx = bc_idx;
     newtable[root->ac_lsigs - 1] = lsig;
     root->ac_lsigtable = newtable;
 
@@ -1289,7 +1291,7 @@ static int cli_loadldb(FILE *fs, struct cl_engine *engine, unsigned int *signo,
 	ret = load_oneldb(buffer,
 			  engine->pua_cats && (options & CL_DB_PUA_MODE) && (options & (CL_DB_PUA_INCLUDE | CL_DB_PUA_EXCLUDE)),
 			  !!engine->ignored,
-			  engine, options, dbname, line, &sigs, NULL, buffer_cpy);
+			  engine, options, dbname, line, &sigs, 0, buffer_cpy);
 	if (ret)
 	    break;
     }
@@ -1335,7 +1337,6 @@ static int cli_loadcbc(FILE *fs, struct cl_engine *engine, unsigned int *signo,
     }
     bcs->count++;
     bc = &bcs->all_bcs[bcs->count-1];
-    bc->id = bcs->count;
 
     switch (engine->bytecode_security) {
 	case CL_BYTECODE_TRUST_ALL:
@@ -1354,6 +1355,7 @@ static int cli_loadcbc(FILE *fs, struct cl_engine *engine, unsigned int *signo,
 	cli_errmsg("Unable to load %s bytecode: %s\n", dbname, cl_strerror(rc));
 	return rc;
     }
+    bc->id = bcs->count;/* must set after _load, since load zeroes */
     sigs += 2;/* the bytecode itself and the logical sig */
     if (bc->kind == BC_LOGICAL || bc->lsig) {
 	if (!bc->lsig) {
@@ -1361,7 +1363,7 @@ static int cli_loadcbc(FILE *fs, struct cl_engine *engine, unsigned int *signo,
 	    return CL_EMALFDB;
 	}
 	cli_dbgmsg("Bytecode %s(%u) has logical signature: %s\n", dbname, bc->id, bc->lsig);
-	rc = load_oneldb(bc->lsig, 0, 0, engine, options, dbname, 0, &sigs, bc, NULL);
+	rc = load_oneldb(bc->lsig, 0, 0, engine, options, dbname, 0, &sigs, bcs->count, NULL);
 	if (rc != CL_SUCCESS) {
 	    cli_errmsg("Problem parsing logical signature %s for bytecode %s: %s\n",
 		       bc->lsig, dbname, cl_strerror(rc));