FIAT DAO veFDT contest - rokinot's results

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/main/contracts/features/Blocklist.sol#L37

Vulnerability details

Impact

In order to avoid being blocked, a malicious contracts can frontrun the block() function by calling selfdestruct().

The selfdestruct(address) function ejects the ether to the set address and wipes the contract's bytecode, which makes the block function revert. The contract can resurrect back to the same address as long as it uses CREATE2 with the same parameters as it's original deployment (assuming it was initially deployed using the same function), sandwiching the blocklist function call.

Note that the contract doesn't lose the lock nor it's delegation in the process.

Proof of Concept

• Malicious contract is deployed using create2
• The malicious contract commits self destruction, which deletes it's byte code
• block() is called, but this function returns false
• block() fails due to the requirement check here
• The contract can be redeployed to the same address using create2 as long as the same initial parameters are used (salt, no code changes)
• Delegation is still set to the contract's address. All non-Ether assets deposited in the contract (ERC20s, ERC721s, etc) will still be there as well.

Tools Used

Code reading

Low

Missing zero checks for important operations

Both contract's constructors are missing zero address checks for their deployment.

#L100-L122 #L14-L17

Non-critical

Missing dev parameters

#L170

Variables only set at the constructor should use the immutable keyword

#L64-L66 #L45 #L16

More efficient struct ordering

The LockedBalance struct can consume 3 storage slots instead of 4 with the following ordering: {int128,int128,uint256,address}

#L76-L79

Functions guaranteed to revert if called by normal users can save gas by adding the payable keyword

#L139 #L170

uEpoch + 1 should be saved in a local variable in order to save gas

#L258-L264

x = x + y is a more efficient operation than x += y

#L418 #L420 #L460-L461 #L465 #L472 #L603 #L654

use ++i for loop increments rather than i++

#L717 #L739