code-423n4 / 2022-04-jpegd-findings

1 stars 1 forks source link

Insecure transfer can lead to mint/burn tokens incorrectly #206

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Users could burn/mint JPEG staking tokens for free

Proof of Concept

I dont know if this should be reported as one issue or two.

Both stake() and unstake() functions at JPEGStaking.sol are using the insecure transfer() and transferFrom(). The problem is that according EIP-20 standard these functions can return false if the transfer fails. So users could, in some conditions, be burning or minting tokens for free.

I saw that you had already imported OZ library at L12.

using SafeERC20Upgradeable for IERC20Upgradeable;

it is just you called the incorrect transfer function .

Tools Used

Manual code review

Recommended Mitigation Steps

Either use safetransfer() and safetransferFrom() both part of .SafeERC20Upgradeable library or check the output and revert in case of false.

spaghettieth commented 2 years ago

This is not an issue as JPEG reverts in case of a failed transfer.