code-423n4 / 2022-12-prepo-findings

0 stars 1 forks source link

User can burn their tokens outside of redeem #309

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/3d7a93876a2e5e1d7fe29b5a0e96e222afdc4cfa/contracts/token/ERC20/extensions/ERC20Burnable.sol#L20-L22

Vulnerability details

Impact

User can burn their tokens outside of the redeem function and trap collateral

Proof of Concept

function burn(uint256 amount) public virtual {
    _burn(_msgSender(), amount);
}

LongShortToken inherits from OZ's ERC20Burnable. This contains a method that allows users to burn their own tokens. Token burned like this won't be able to claim the underlying collateral in the contract and will leave it trapped there permanently.

Tools Used

Manual Review

Recommended Mitigation Steps

In LongShortToken, override the burn methods to only allow the PrePOMarket to burn tokens:

function burn(uint256 amount) public override onlyOwner {
    _burn(_msgSender(), amount);
}

function burnFrom(address account, uint256 amount) public override onlyOwner {
    _spendAllowance(account, _msgSender(), amount);
    _burn(account, amount);
}
Picodes commented 1 year ago

Downgrading to QA as the same thing can happen if someone transfers LongShortToken to address(0) or any wrong address

c4-judge commented 1 year ago

Picodes changed the severity to QA (Quality Assurance)

c4-judge commented 1 year ago

Picodes marked the issue as grade-b

c4-sponsor commented 1 year ago

davidprepo marked the issue as sponsor disputed

ghost commented 1 year ago

If a user wants to cast their tokens into the void, that's their prerogative