Return value not checked at `transferFrom` call

[code423n4]

Lines of code

https://github.com/code-423n4/2022-04-jpegd/blob/e72861a9ccb707ced9015166fbded5c97c6991b6/contracts/staking/JPEGStaking.sol#L34

Vulnerability details

The ERC20.transferFrom() function returns a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead. Tokens that don’t actually perform the transfer and return false are still counted as a correct transfer.

Vulnerable functions: JPEGStaking.stake NFTVault._openPosition NFTVault.repurchase

Proof Of Concept

Example (JPEGStaking, 34):

    jpeg.transferFrom(msg.sender, address(this), _amount);

    _mint(msg.sender, _amount);

if the jpeg ERC20 implements a transferFrom which return False instead of revert, which is valid as the IERC20Upgradeable interface's transferFrom returns a boolean, then the transfer will be considered succesfull and the contract will continue execution and call _mint() even though the sender did not send any tokens. Similar vulnerability for nftContract which is an IERC721Upgradeable in NFTVault._openPosition, and stablecoin which is an IStableCoin in NFTVault.repurchase

Tools Used

VScode

Recommended Mitigation Steps

Use safeTransferFrom() . This function calls revert instead of returning a boolean

[spaghettieth]

JPEG doesn't return false on a failed transfer, it just reverts.

[dmvt]

Invalid. Sponsor wrote the code for the token (and it is included in this contest) so they know that it functions correctly.