6.1 Insecure crate used
Usage of tracing-subscriber package version 0.3.19 is deemed insecure as logging user input may result in poisoning logs with ANSI escape sequences explained in RUSTSEC.
Recommendation
Upgrade to version >=0.3.20.
Resolution
Created patch PR in both celo-kona and op-succinct: PR 78 PR 33
6.2 Duplicate code leading to redundant cycles
In both main.rs and lib.rs/run_range_program(...) the function get_oracle_and_blob_provider(...) is called to create an oracle and a blob store. This means the same code is executed twice and can thus slow down the proving time of the code.
Recommendation
Editing the function run_range_program(...) to take as parameters the oracle and blob store, as this would mean that it would not need to be recalculated.
Resolution
Included in the following
6.3 Gas behavior differs if precompile is used outside GoldToken
The transfer precompile’s run(...) function restores the warmness of the to address after performing a balance transfer but does not restore the warmness of the from address. In the current system, this behavior is correct and has no side effects because the precompile is always invoked through the GoldToken contract.
Before calling the precompile, GoldToken performs a balance check on the from address, which triggers the EVM BALANCE opcode:
function balanceOf(address owner) public view returns (uint256) {
//@audit BALANCE opcode warms `owner`
return owner.balance;
}
According to EVM semantics, any address accessed via the BALANCE opcode is automatically added to the access list and becomes warm. Therefore, by the time the precompile executes, the from account is already warm, and restoring its warmness state is unnecessary.
However, this assumption only holds as long as the precompile is invoked through GoldToken. If the precompile were to be called directly by another contract or address that does not access from beforehand, the from address could start as cold. In such a case, the internal journal.transfer(...) call would mark it warm and alter gas metering for the remainder of the transaction.
Recommendation
Consider documenting that the from address is guaranteed to be warm when the precompile is called through GoldToken.
Resolution
Made a PR for it and merged PR 77.
6.4 Missing STATICCALL guard allows state-changing precompile to be invoked in static context
The transfer_run(...) flow exposes a state-changing precompile that debits and credits balances through journal().transfer(...). This precompile is intended to be called only by the token contract in standard, non-static execution, and it charges gas before returning. However, the implementation does not verify whether the call is being executed under STATICCALL (static context). In static context, EVM semantics prohibit state changes. Despite that, the code proceeds to execute the transfer, which attempts to mutate state.
fn run<CTX>(
context: &mut CTX,
input: &Bytes,
caller_address: Address,
gas_limit: u64,
) -> PrecompileResult
where
CTX: ContextTr<Cfg: Cfg<Spec = OpSpecId>>,
{
// ...
if input.len() != 96 {
return Err(PrecompileError::Other("invalid input length".to_string()));
}
// ...
// @audit This call mutates state but there is no STATICCALL guard before it.
let result = context.journal().transfer(from, to, value);
// ...
}
Recommendation
Consider introducing a strict check that rejects execution when the precompile is invoked in a static context. The precompile should exit early with an error indicating that state changes are not allowed during static calls.
Resolution
Opened a PR PR 79.
