search

code-423n4 / 2022-05-velodrome-findings

0 stars 0 forks source link

QA Report #12

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

No Transfer Ownership Pattern

description

The current minter transfer process involves the current minter calling setMinter(). If the nominated EOA account is not a valid account, it is entirely possible the owner may accidentally transfer ownership to an uncontrolled account, or address(0), breaking all functions that requre msg.sender == _minter

Recommend considering implementing a two step process where the minter nominates an account and the nominated account needs to call an accept() function for the transfer of minter to fully succeed. This ensures the nominated EOA account is a valid and active account.

findings

/2022-05-velodrome/contracts/contracts/Velo.sol
26:     function setMinter(address _minter) external {
27:         require(msg.sender == minter);
28:         minter = _minter;
29:     }

the setVoter function in VotingEscrow also does not implement this pattern

/2022-05-velodrome/contracts/contracts/VotingEscrow.sol
1059:     function setVoter(address _voter) external {
1060:         require(msg.sender == voter);
1061:         voter = _voter;
1062:     }

open TODO

description

clean up any open TODOs in the comments

findings

/2022-05-velodrome/contracts/contracts/VotingEscrow.sol
314: // TODO delegates
465: // TODO add delegates
524: // TODO add delegates
/2022-05-velodrome/contracts/contracts/VelodromeLibrary.sol
9: IRouter internal immutable router; // TODO make modifiable?
GalloDaSballo commented 2 years ago

No Transfer Ownership Pattern

NC

open TODO

NC