code-423n4 / 2021-05-fairside-findings

0 stars 0 forks source link

`ERC20ConvictionScore.tokenizeConviction` transfers locked balance from user twice #44

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Handle

cmichel

Vulnerability details

Vulnerability Details

In tokenizeConviction when locked > 0 the amount is first transferred from the user using an internal call to _transfer(msg.sender, address(fairSideConviction), locked);. It is then transferred a second time from the user in the fairSideConviction.createConvictionNFT call:

function createConvictionNFT(
    address user,
    uint256 score,
    uint256 locked,
    bool isGovernance
) external override returns (uint256) {
    if (locked > 0) {
        cs.locked = locked;
        // steals a second time
        FSD.safeTransferFrom(user, address(this), locked);
    }
}

Impact

The locked balance is transferred twice from the user instead of once, stealing their balance.

Recommended Mitigation Steps

Remove the transfer in createConvictionNFT.

fairside-core commented 3 years ago

Duplicate of #74

cemozerr commented 3 years ago

Duplicate of #29