minhquanym commented 2 weeks ago

[M-02] Users can exploit the system by depositing 1 wei and only depositing more if they roll a favorable number

Severity

Impact: Medium

Likelihood: Medium

Description

In the LotteryV2Base contract, buyers can roll a dice to generate a random number. Those who roll numbers close to the seller's number become eligible for minting.

Each roll requires a rollPrice. However, users receive a "free" roll in the deposit() function. This can be exploited by an attacker. The attacker might only deposit 1 wei into the contract to get this "free" roll, and then deposit the necessary amount to claim the number if they roll a winning number. If they don't roll a winning number, they can simply create and deposit into a new account/address.

function deposit(uint256 amount) public lotteryStarted hasNotWonInLotteryV1(_msgSender()) {
    require(usdcContractAddr != address(0), "USDC contract address not set");
    require(amount > 0, "No funds sent");
    require(
        IERC20(usdcContractAddr).allowance(_msgSender(), address(this)) >= amount,
        "Insufficient allowance"
    );

    IERC20(usdcContractAddr).transferFrom(_msgSender(), address(this), amount);

    if (deposits[_msgSender()] == 0) {
        participants.push(_msgSender());
    }
    deposits[_msgSender()] += amount;

    // @audit Free roll
    if (rolledNumbers[_msgSender()] == 0) {
        _requestRandomness(abi.encode(_msgSender()));
        emit RandomRequested(_msgSender());
    }
}

Recommendations

Consider charging the rollPrice in the deposit() function.

0ximmeas commented 1 week ago

I had thought a lot about this one. but in the end, i'm leaning towards this being invalid: the buyer still bought a ticket for the full price. Compare it to a user that directly deposited the ticket amount, if that user wins, they'll pay the full ticket amount (same). If they lose, they can still withdraw their full deposit (same). Hence there is no difference between paying dust upfront then topping up, compared to paying everything upfront.

i agree though that this might not be what the protocol intended, perhaps low severity?

minhquanym commented 1 week ago

It is different. Imagine I created 1000 accounts with only 1000 weis => It will definitely increase my chance of winning without needing the capital to buy 1000 tickets. Btw, I think it has the same impact with this finding though https://github.com/CDSecurity/Blessed-Immeas/issues/5

0ximmeas commented 1 week ago

Ah, yes, you're right. It lowers the bar of entry.

CDSecurity / Blessed-minhquanym