User can burn their tokens outside of redeem

[code423n4]

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]

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

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-b

[c4-sponsor]

davidprepo marked the issue as sponsor disputed

[ghost]

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