Omniscia Gravita Protocol Audit
Timelock Manual Review Findings
Timelock Manual Review Findings
TKC-01M: Inexistent Prevention of Duplicate Invocations
| Type | Severity | Location |
|---|---|---|
| Logical Fault |  | Timelock.sol:L106-L125, L127-L138 |
Description:
Based on the implementation of the Gravita Protocol codebase, the Timelock contract is expected to be managed by an EOA / multi-signature wallet rather than an on-chain decentralized smart contract. As such, calls to it aren't restricted similarly to how DAOs prevent the same payload to be queued again.
Impact:
It is presently possible to emit events that do not correspond to the real state of the Timelock, cancelling a transaction that has already been executed thus breaking the guarantee that a CancelTransaction event is meant to indicate the transaction has not been executed and has been cancelled.
Example:
127function cancelTransaction(128 address target,129 uint value,130 string memory signature,131 bytes memory data,132 uint eta133) public adminOnly {134 bytes32 txHash = keccak256(abi.encode(target, value, signature, data, eta));135 queuedTransactions[txHash] = false;136
137 emit CancelTransaction(txHash, target, value, signature, data, eta);138}139
140function executeTransaction(141 address target,142 uint value,143 string memory signature,144 bytes memory data,145 uint eta146) public payable adminOnly returns (bytes memory) {147 bytes32 txHash = keccak256(abi.encode(target, value, signature, data, eta));148 if (!queuedTransactions[txHash]) {149 revert Timelock__TxNoQueued();150 }151 if (getBlockTimestamp() < eta) {152 revert Timelock__TxStillLocked();153 }154 if (getBlockTimestamp() > eta + GRACE_PERIOD) {155 revert Timelock__TxExpired();156 }157
158 queuedTransactions[txHash] = false;159
160 bytes memory callData;161
162 if (bytes(signature).length == 0) {163 callData = data;164 } else {165 callData = abi.encodePacked(bytes4(keccak256(bytes(signature))), data);166 }167
168 // Execute the call169 (bool success, bytes memory returnData) = target.call{ value: value }(callData);170 if (!success) {171 revert Timelock__TxReverted();172 }173
174 emit ExecuteTransaction(txHash, target, value, signature, data, eta);175
176 return returnData;177}Recommendation:
We advise the code of transaction queueing and transaction cancelling to prevent execution if the transaction is already queued or already cancelled respectively. This will prevent misleading QueueTransaction and CancelTransaction events from being emitted, such as a transaction actually being executed by Timelock::executeTransaction and then "cancelled" by Timelock::cancelTransaction even though it has already been executed.
Alleviation:
The queue status of a transaction is now sanitized in all statements that adjust it, ensuring that it solely transitions from an unqueued to a queued state and vice versa.