Vultisig should be burnable

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-vultisig/blob/cb72b1e9053c02a58d874ff376359a83dc3f0742/hardhat-vultisig/contracts/Vultisig.sol#L1-L25

Vulnerability details

The Vultisig token, as described in its documentation, is expected to include a burnable feature. However, the current implementation of the Vultisig token contract lacks the necessary functions to support token burning. This report identifies the impact of this missing functionality and provides a recommended solution to implement the burn feature. The vultisig stated that they forgot to add this functionality.

Impact

Non-compliance with Documentation: Users and developers relying on the documentation will expect burn functionality, leading to confusion and potential loss of trust when they find it missing.

Proof of concept

As you can see from the code below function for burning is missing, consider adding it to the code.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IApproveAndCallReceiver} from "./interfaces/IApproveAndCallReceiver.sol";

/**
 * @title ERC20 based Vultisig token contract
 */
contract Vultisig is ERC20, Ownable {
    constructor() ERC20("Vultisig Token", "VULT") {
        _mint(_msgSender(), 100_000_000 * 1e18);
    }

    function approveAndCall(
        address spender,
        uint256 amount,
        bytes calldata extraData
    ) external returns (bool) {
        // Approve the spender to spend the tokens
        _approve(msg.sender, spender, amount);

        // Call the receiveApproval function on the spender contract
        IApproveAndCallReceiver(spender).receiveApproval(
            msg.sender,
            amount,
            address(this),
            extraData
        );

        return true;
    }
}

Recommended Mitigation Steps

To address this issue, the following burn functions should be added to the Vultisig contract:

1. Burn Function:

   • Allows token holders to destroy a specified amount of their own tokens.

2. Burn From Function:

   • Allows an account to burn tokens from another account, given that the caller has sufficient allowance.

Here is the modified contract with the added burn functionality: Something like this can be added to the Vultisig code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.24;

import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";
import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
import {IApproveAndCallReceiver} from "./interfaces/IApproveAndCallReceiver.sol";

/**
 * @title ERC20 based Vultisig token contract
 */
contract Vultisig is ERC20, Ownable {
    constructor() ERC20("Vultisig Token", "VULT") {
        _mint(_msgSender(), 100_000_000 * 1e18);
    }

    function approveAndCall(
        address spender,
        uint256 amount,
        bytes calldata extraData
    ) external returns (bool) {
        // Approve the spender to spend the tokens
        _approve(msg.sender, spender, amount);

        // Call the receiveApproval function on the spender contract
        IApproveAndCallReceiver(spender).receiveApproval(
            msg.sender,
            amount,
            address(this),
            extraData
        );

        return true;
    }

+    function burn(uint256 amount) public {
+        _burn(msg.sender, amount);
+    }

+    function burnFrom(address account, uint256 amount) public {
+        uint256 currentAllowance = allowance(account, msg.sender);
+        require(currentAllowance >= amount, "ERC20: burn amount exceeds + allowance");
+        _approve(account, msg.sender, currentAllowance - amount);
+        _burn(account, amount);
+    }
}

Assessed type

ERC20

[c4-judge]

alex-ppg marked the issue as selected for report

[c4-judge]

alex-ppg marked the issue as satisfactory

[c4-judge]

alex-ppg marked the issue as primary issue

[alex-ppg]

Per the original discussions in the validation repository, this finding's set was deemed as a valid medium-risk vulnerability due to being a feature described in the documentation that the Sponsor intends to introduce after the contest.

A medium severity was assessed because the functionality is not imperative to the way the protocol works (i.e. all contracts behave "as expected" without it), and burning functionality can be replicated by f.e. transferring funds to the 0xdeaD...DEaD address.

[c4-judge]

alex-ppg changed the severity to 2 (Med Risk)

[thebrittfactor]

Vultisig sponsors (paaao and 0xtrips) confirmed via outside Github that the issue was confirmed and implemented.