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

0 stars 0 forks source link

Locked funds are debited twice from user during tokenization leading to fund loss #29

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

0xRajeev

Vulnerability details

Impact

During tokenization of conviction scores, the user can optionally provide FSDs to be locked to let it continue conviction accrual. However, the amount of FSDs specified for locking are debited twice from the user leading to fund loss for user.

This, in effect, forces the user to unknowingly and unintentionally lock twice the amount of FSD tokens, leading to a loss of the specified ‘locked’ number of tokens.

Proof of Concept

Alice decides to tokenise her conviction score into an NFT and specifies 100 FSD tokens to be locked in her call to tokenizeConviction(100). 100 FSD tokens are transferred from her FSD balance to FairSideConviction contract on Line282 of ERC20ConvictionScore.sol. However, in FairSideConviction.createConvictionNFT(), the specified locked amount is transferred again from Alice to the contract on Line50 of FairSideConviction.sol.

The impact is that Alice wanted to lock only 100 FSD token but the FairSide protocol has debited 200 tokens from her balance leading to a loss of 100 FSD tokens.

https://github.com/code-423n4/2021-05-FairSide/blob/3e9f6d40f70feb67743bdc70d7db9f5e3a1c3c96/contracts/dependencies/ERC20ConvictionScore.sol#L282

https://github.com/code-423n4/2021-05-FairSide/blob/3e9f6d40f70feb67743bdc70d7db9f5e3a1c3c96/contracts/conviction/FairSideConviction.sol#L48-L51

Tools Used

Manual Analysis

Recommended Mitigation Steps

Remove the redundant transfer of FSD tokens on Line282 in tokenizeConviction() of ERC20ConvictionScore.sol.

fairside-core commented 3 years ago

Duplicate of #74