smartcontractkit / smartcontractkit/chainlink-aptos

`burn(...)` function would enable burners to burn users assets without their consent

Open
#222 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Move
Stars
10
Forks
14
Avg merge
1d 7h
Merged PRs (30d)
29

Description

Hi team
I'm studying your implementation to learn Aptos Move development and noticed what appears to be a significant difference from standard token burning patterns. Your burn function allows privileged addresses to burn tokens from any user's account without their explicit consent or approval.
current implementation :

public entry fun burn(
        burner: &signer, from: address, amount: u64
    ) acquires TokenMetadataRefs, TokenState {
        let burner_addr = signer::address_of(burner);
        assert_is_allowed_burner(burner_addr);

        if (amount == 0) { return };

        primary_fungible_store::burn(
            &borrow_token_metadata_refs().burn_ref, from, amount
        ); 

        event::emit(Burn { burner: burner_addr, from, amount });
    }

The test even clearly has it

   #[test(owner = @0x999, recipient = @0xcafe, managed_token = @managed_token)]
    public fun test_burn_managed_token(
        owner: &signer, recipient: &signer, managed_token: &signer
    ) {
        // Setup env and mint managed_token first, mint 100 to recipient
        test_mint_managed_token(owner, recipient, managed_token);

        let recipient_addr = signer::address_of(recipient);
        let burn_amount: u64 = 50;

        managed_token::burn(owner, recipient_addr, burn_amount);

        let metadata_obj =
            object::address_to_object<Metadata>(managed_token::token_metadata());
        // 100 is the mint amount, 50 is the burn amount
        let mint_amount: u64 = 100;
        assert!(
            primary_fungible_store::balance(recipient_addr, metadata_obj)
                == mint_amount - burn_amount
        );

        // Assert tokens are burned from existing supply
        assert!(
            fungible_asset::supply(metadata_obj)
                == option::some((mint_amount - burn_amount) as u128)
        );
    }

This implementation enables any address with "burner" privileges to burn tokens from any user's account (as shown in the test where owner burns tokens from recipient_addr). This is a significant deviation from standard token burning patterns.

In OpenZeppelin's ERC20 implementation:

  • burn() only burns caller's own tokens
  • burnFrom() requires explicit allowance from the token owner
// OpenZeppelin Contracts (last updated v5.0.0) (token/ERC20/extensions/ERC20Burnable.sol)

    /**
     * @dev Destroys a `value` amount of tokens from the caller.
     *
     * See {ERC20-_burn}.
     */
    function burn(uint256 value) public virtual {
        _burn(_msgSender(), value);
    }
    /**
     * @dev Destroys a `value` amount of 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
     * `value`.
     */
    function burnFrom(address account, uint256 value) public virtual {
        _spendAllowance(account, _msgSender(), value);
        _burn(account, value);
    }

I'm wondering if the move implementation is a design choice so I would really appreciate any clarification

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reviewing the public burn entry function and the test_burn_managed_token test shown in the issue, then compare their authorization behavior with the project's intended token model. Done means maintainers have decided whether privileged burning is intended and the behavior, tests, or documentation clearly reflect that decision.

Written by the indexing model from the issue text.

Assessment

Domain
blockchain, security
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.