diff --git a/contrib/codeql/nightly/NonBinaryIsFunction.ql b/contrib/codeql/nightly/NonBinaryIsFunction.ql new file mode 100644 index 00000000000..74943db20f3 --- /dev/null +++ b/contrib/codeql/nightly/NonBinaryIsFunction.ql @@ -0,0 +1,39 @@ +/** + * @name Finds functions that suggest a boolean return type, but do not. + * @description This query identifies functions that are named with the `is` prefix but return values outside + * the typical boolean range of 0 and 1. + * @kind problem + * @id asymmetric-research/non-binary-is-function + * @precision medium + * @severity warning + * @tags correctness + * maintainability + */ + +import cpp +import semmle.code.cpp.rangeanalysis.new.SimpleRangeAnalysis + +/** + * A function whose name suggests it returns a boolean value (0 or 1). + */ +class IsFunction extends Function { + IsFunction() { + this.getName().matches("%\\_is\\_%") and + this.getType() instanceof IntegralType and + not this.getName() = "fd_bn254_pairing_is_one_syscall" + // returns values in [-1, 0] + } +} + +from IsFunction f, ReturnStmt rs, int lowerBound, int upperBound +where + rs.getEnclosingFunction() = f and + lowerBound = lowerBound(rs.getExpr().getFullyConverted()) and + upperBound = upperBound(rs.getExpr().getFullyConverted()) and + ( + lowerBound < 0 or + upperBound > 1 + ) +select rs, + "The function $@ is named like an `is` function but returns a value with bounds [" + lowerBound + + ", " + upperBound + "].", f, f.getName() diff --git a/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.c b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.c new file mode 100644 index 00000000000..b1c968be865 --- /dev/null +++ b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.c @@ -0,0 +1,44 @@ +// Add missing typedef and define +#define FD_EXECUTOR_INSTR_ERR_MISSING_ACC (-33) /* An account required by the instruction is missing */ +#define FD_BANK_FLAGS_DEAD (0x00000008UL) /* Dead. We stopped replaying it before we could finish it (e.g. invalid block or pruned minority fork). */ + +typedef struct +{ + int acct_cnt; + struct + { + int is_signer; + } accounts[10]; +} fd_instr_info_t; + +typedef struct +{ + int flags; +} fd_bank_t; + +int fd_instr_acc_is_signer_idx(fd_instr_info_t const *instr, + short idx) +{ + if (FD_UNLIKELY(idx >= instr->acct_cnt)) + { + return FD_EXECUTOR_INSTR_ERR_MISSING_ACC; // $ Alert (returns -33 which is not 0 or 1) + } + + return !!(instr->accounts[idx].is_signer); +} + +static inline int +fd_banks_is_bank_dead(fd_bank_t *bank) +{ + return bank->flags & FD_BANK_FLAGS_DEAD; // $ Alert (returns value in [0, 8]) +} + +// inspired by the function, but heavily simplified +int fd_bn254_pairing_is_one_syscall(unsigned long in_sz) +{ + if (in_sz != 128UL) + { + return -1; // No alert + } + return 0; +} \ No newline at end of file diff --git a/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.expected b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.expected new file mode 100644 index 00000000000..844e363cd0f --- /dev/null +++ b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.expected @@ -0,0 +1,2 @@ +| NonBinaryIsFunction.c:24:9:24:49 | return ... | The function $@ is named like an `is` function but returns a value with bounds [-33, -33]. | NonBinaryIsFunction.c:19:5:19:30 | fd_instr_acc_is_signer_idx | fd_instr_acc_is_signer_idx | +| NonBinaryIsFunction.c:33:5:33:44 | return ... | The function $@ is named like an `is` function but returns a value with bounds [0, 8]. | NonBinaryIsFunction.c:31:1:31:21 | fd_banks_is_bank_dead | fd_banks_is_bank_dead | diff --git a/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.qlref b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.qlref new file mode 100644 index 00000000000..79e6810c2fe --- /dev/null +++ b/contrib/codeql/test/query-tests/NonBinaryIsFunction/NonBinaryIsFunction.qlref @@ -0,0 +1,2 @@ +query: NonBinaryIsFunction.ql +postprocess: InlineExpectationsTestQuery.ql \ No newline at end of file