Canto v2 contest - Limbooo's results

(low) ERC20 allowance front-running issue.

Governance/Comp.sol is an ERC20 and it is not preventing allowance front-running issue, approving tokens to other accounts might be vulnerable to the front-running attack. more info here

PoC

in Governance/Comp.sol#approve() if there is non empty value in allowances[account][spender] the spender can use the 2 of the approvment even if the original account didn't want this.

Recommendation

Define increaseAllowance and decreaseAllowance as ERC20 implementation. or revert when allowances[account][spender] has a value only if the amount approved != 0 so the approver can cancel his approvement.

(non) Redundant check instead of new admin address check

When use msg.sender == address(0) its actually never would be true in every cases. I believe it's a mistake and it's meant to revert when updatedAddress == address(0).

PoC

when pendingAdmin call CToken.sol#_acceptAdmin() and Unitroller.sol#_acceptAdmin() will take the pendingAdmin from storage and assign it as admin. But when contract initialize pendingAdmin will be address(0) and this is never checked either in CToken.sol#_setPendingAdmin() nor in Unitroller.sol#_setPendingAdmin().

Recommendation

In these lines: CToken.sol#_acceptAdmin() and Unitroller.sol#_acceptAdmin()

// Check caller is pendingAdmin and pendingAdmin ≠ address(0)
if (msg.sender != pendingAdmin || msg.sender == address(0)) {
    ...

I believe it meant to be

if (msg.sender != pendingAdmin) {
    ...

or

if (msg.sender != admin && pendingAdmin == address(0)) {
    ...