Users can escape the burnFrom function by transferring rsETH to another account or selling it on the open market

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-11-kelp/blob/f751d7594051c0766c7ecd1e68daeb0661e43ee3/src/RSETH.sol#L54-L56 https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/release-v4.7/contracts/token/ERC20/ERC20Upgradeable.sol#L1

Vulnerability details

Proof of Concept

rsETH is an upgradeable ERC20 contract. The mint function is exposed but the burn function is not, meaning that the users cannot burn their rsETH. However, there is a burnFrom function in rsETH, where the admin with the BURNER_ROLE can burn rsETH from any account.

RSETH.sol
    function burnFrom(address account, uint256 amount) external onlyRole(BURNER_ROLE) whenNotPaused {
        _burn(account, amount);
    }

Should the admin intend to burn from an account, that account has 3 options to protect their rsETH.

1. Frontrun the admin by transferring their rsETH to another account using the publicly exposed transfer function in the ERC20 Upgradeable contract.

ERC20Upgradeable.sol
    function transfer(address to, uint256 amount) public virtual override returns (bool) {
        address owner = _msgSender();
        _transfer(owner, to, amount);
        return true;
    }

2. Frontrun the admin and trade the rsETH off on the open market.
3. Approve another account/accounts beforehand. Frontrun the admin by using the transferFrom method.

ERC20Upgradeable.sol
    function approve(address spender, uint256 amount) public virtual override returns (bool) {
        address owner = _msgSender();
        _approve(owner, spender, amount);
        return true;
    }

(Assume OpenZeppelin version <5.0)

Impact

Users can escape burning in the event that the admin intends to burn rsETH from an account.

Tools Used

Manual Review

Recommended Mitigation Steps

A better way to prevent users from escaping the burn is to blacklist the potential accounts that has rsETH that needs to be burned. Then, restrict their functionality through the token transfer hooks or overriding the new _update function (if using Openzeppelin 5.0 and up). Restrict the transfer and approve function so that the blacklisted accounts cannot transfer their tokens away, trade the rsETH off or approve another account.

Assessed type

ERC20

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #43

[c4-judge]

fatherGoose1 marked the issue as unsatisfactory: Invalid

[cryptostaker2]

Hi @fatherGoose1, this is not a duplicate of #43, whereby the withdrawal function is missing.

This is a case of escaping the admin burnFrom function, whereby users can evade the burn by frontrunning the admin (as stated in this issue).

A similar issue that was previously judged as medium is the Ethena M-01 issue: stakers can bypass restrictions, issue 499.

Thanks for your time and effort!

[fatherGoose1]

@cryptostaker2 I was the judge for Ethena. Ethena imposed restrictions to avoid legal disputes. No such safeguards are in place for Kelp, and front-running a burnFrom() call is possible for all tokens.