068.md

Chilly Rose Hornet

Medium

External Call Status Check in drawRaffle Function

Summary

The ERC20Incentive::drawRaffle function does not check the status of the safeTransfer call. Although safeTransfer is designed to revert on failure, it is a best practice to explicitly check the return status to ensure that the transfer was successful. Failure to do so can lead to unnoticed failed transfers, causing inconsistencies in the contract state.

Vulnerability Detail

The function does not verify the success of the safeTransfer call. This oversight can result in failed transfers going unnoticed, leading to potential inconsistencies in the contract state.

Impact

If the safeTransfer call fails and the failure is not explicitly checked, the contract may proceed as if the transfer was successful, which can cause discrepancies in the contract's state, leading to financial losses or incorrect data being stored.

Code Snippet

https://github.com/sherlock-audit/2024-06-boost-aa-wallet/blob/main/boost-protocol/packages/evm/contracts/incentives/ERC20Incentive.sol#L144-L145

Proof of Concept

This test case demonstrates the vulnerability by mocking a failed transfer and showing that the Claimed event is still emitted.

const { expect } = require("chai");
const { ethers } = require("hardhat");

describe("Raffle Contract", function () {
    let Raffle, raffle, owner, addr1, addr2, asset;

    beforeEach(async function () {
        [owner, addr1, addr2, ...addrs] = await ethers.getSigners();
        const Asset = await ethers.getContractFactory("MockERC20");
        asset = await Asset.deploy("Mock Asset", "MA", 1000);
        await asset.deployed();

        Raffle = await ethers.getContractFactory("Raffle");
        raffle = await Raffle.deploy(asset.address, 100);
        await raffle.deployed();
    });

    it("Should emit Claimed event even if transfer fails", async function () {
        // Add entries
        await raffle.connect(owner).addEntry(addr1.address);
        await raffle.connect(owner).addEntry(addr2.address);

        // Set strategy to RAFFLE
        await raffle.connect(owner).setStrategy(1); // Assuming 1 is RAFFLE

        // Mock transfer to fail by not approving the raffle contract to transfer tokens
        await asset.connect(owner).approve(raffle.address, 0);

        // Draw raffle and expect the Claimed event to be emitted despite transfer failure
        await expect(raffle.connect(owner).drawRaffle())
            .to.emit(raffle, "Claimed")
            .withArgs(addr1.address, ethers.utils.defaultAbiCoder.encode(["address", "address", "uint256"], [asset.address, addr1.address, 100]));
    });
});

Tool used

Manual Review

Recommendation

Explicitly check the return status of the safeTransfer call and handle any failures appropriately.