Conflicting Burn Mechanisms

Summary

The Ajna token has conflicting method which could potentially ruin the accounting mechanism adopted by the protocol. The AjnaToken.sol implements the ERC20Burnable as well as a seperately BurnWrappedAjna

Vulnerability Detail

The AjnaToken already implements burnability of tokens by the owner or an approved address. This burns the Tokens and reduces the total supply. The BurnWrappedAjna however implements a burnable wrapper for ajna, which locks the tokens deposited in the contract forever . See

    function withdrawTo(address, uint256) public pure override returns (bool) {
        revert UnwrapNotAllowed();
    }

It also mints a burnWrapped token for the protocol when tokens are deposited. But Ajna tokens in this contract can never be burnt (Only the wrapped tokens can be burnt), therefore BurnWrapping your tokens do not decrease the ajna token total supply.

Impact

This leads to conflicting TVLs, token permanently locked and other unintended outcomes

Code Snippet

https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-grants/src/token/AjnaToken.sol#L11-L15 ERC20Burnable.sol we have

abstract contract ERC20Burnable is Context, ERC20 {
    /**
     * @dev Destroys `amount` tokens from the caller.
     *
     * See {ERC20-_burn}.
     */
    function burn(uint256 amount) public virtual {
        _burn(_msgSender(), amount);
    }

    /**
     * @dev Destroys `amount` tokens from `account`, deducting from the caller's
     * allowance.
     *
     * See {ERC20-_burn} and {ERC20-allowance}.
     *
     * Requirements:
     *
     * - the caller must have allowance for ``accounts``'s tokens of at least
     * `amount`.
     */
    function burnFrom(address account, uint256 amount) public virtual {
        _spendAllowance(account, _msgSender(), amount);
        _burn(account, amount);
    }

https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-grants/src/token/BurnWrapper.sol#L9-L19 https://github.com/sherlock-audit/2023-04-ajna/blob/main/ajna-grants/src/token/BurnWrapper.sol#L56-L58

Tool used

Manual Review

Recommendation

Use only one burning mechanism to achieve the results specified in the white paper

grandizzy commented 1 year ago

This contract is meant to be used only on sidechains and not on rollups and there's no risk to users, hence disputing the severity as low / informational. Re conflicting burn mechanisms - there are pros and cons for this approach, depending how these tokens are used

0xffff11 commented 1 year ago

Agree with sponsors explanation. I think it should keep as low severity

sherlock-audit / 2023-04-ajna-judging

josephdara - Conflicting Burn Mechanisms #79