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

2 stars 1 forks source link

Deposit does not verify if tokens are transfered successfully #261

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L426 https://github.com/code-423n4/2022-08-fiatdao/blob/fece3bdb79ccacb501099c24b60312cd0b2e4bb2/contracts/VotingEscrow.sol#L486

Vulnerability details

please find below one report encapsulating 2 M bugs which are the exact same bug found in two seperate functions within the same .sol file

M1 No Return Value on transferFrom

VotingEscrow.sol makes checks to be sure that the transferFrom function is sending to the correct address but does not check of the tokens have actually been transfered or not, there should be some kind of bool return value, but there is not, maybe implement the use of SafeTransfer, or add the bool return value so show the transaction success.

lines 424-428

M2 No Return Value on transferFrom

VotingEscrow.sol makes checks to be sure that the transferFrom function is sending to the correct address but does not check of the tokens have actually been transfered or not, there should be some kind of bool return value, but there is not, maybe implement the use of SafeTransfer, or add the bool return value so show the transaction success.

lines 485-488

Proof of Concept

Alice decide to deposit the locked tokens, she implements the code to transferFrom exactly as the dev intended but unknown to alice her transaction fails not due to the wrong address but for some other reason(maybe the token contract she is interacting with isnt the correct type) Alice wont know this as there is no return value to inform her that the transaction failed possibly leading to the loose/lockup of funds .

Tools Used

Visual studio code Static analysis

Recommended Mitigation Steps

Add the return value to the function and/or implement the use of safeTransferFrom

lacoop6tu commented 2 years ago

Duplicate of #231