code-423n4 / 2022-08-fiatdao-findings

2 stars 1 forks source link

QA Report #250

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

VotingEscrow.transferOwnership() should be a two step process

The mentioned function should perform a two step ownership transfer to prevent an irreversible lost of the ownership in case of a mistakenly input address while calling VotingEscrow.transferOwnership().

VotingEscrow.unlock() should have an interlock or an additional call security measure

If the mentioned function is mistakenly called, the whole logic of the voting, penalties and locking system can immediately start misbehave in an irreversible way.

To prevent having further issues with such an irreversible call, I would consider either making this a two step process or maybe adding an input "magic number" that needs to be given in order to ensure that this function is meant to be called (as a way of interlock). For example:

function unlock(uint256 _interlockNumber) external {
    require(_interlockNumber == 12092018, "Wrong interlock number provided");
    require(msg.sender == owner, "Only owner");
    maxPenalty = 0;
    emit Unlock();
}