Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unchecked return value could lead to loss of funcds

0xodus

Summary

transferFrom functon returns a bool which shows whether a transfer was successful or not, however in some instances in the TokenDivider.sol contract, this is ignored

Vulnerability Details

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/main/src/TokenDivider.sol#L207
https://github.com/Cyfrin/2025-01-pieces-protocol/blob/main/src/TokenDivider.sol#L248
https://github.com/Cyfrin/2025-01-pieces-protocol/blob/main/src/TokenDivider.sol#L305

In the above instances, the return value of transferFrom function is not checked. This means that if the user calls the sellErc20 function and the transferFrom function fails, the functon doesn't revert instead it assumes the transfer went through, this could lead to losses for the user and in other instances the protocol, therefore causing inconsistencies in the contract state.

Impact

The above vulnerability could lead to losses for the protocol and user

Tools Used

Manual Review

Recommendations

Add the following if loop to check if the transfer was successful, and if not, revert

- IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,address(this), amount);
+ bool success = IERC20(tokenInfo.erc20Address).transferFrom(msg.sender,address(this), amount);
+ if(!success) {
+ revert TokenDivider__TransferFailed();
+ }
Updates

Lead Judging Commences

fishy Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.