Boot Finance contest - elprofesor's results

Handle

elprofesor

Vulnerability details

HIGH

Impact

Due to improper validation of input, approved airdrop users are able to double spend airdrop allocated tokens. This is due to insufficient validation in validate() and claimExact() which allows the user to reset the amount of tokens they have claimed.

Proof of Concept

Attacker Executes the following sequence of transactions:

1. validate(): This sets their airdrop[msg.sender].amount to the amount hardcoded in the airdropBalances
2. repeatedly call claim(): each epoch the user should call claim() in order to transfer their Airdrop and drop their `Airdrop[msg.sender].amount close to zero
3. claimExact(): function claim() has the following code assert(airdrop[msg.sender].amount - claimable != 0); and validate() has the following require(airdrop[msg.sender].amount == 0, "Already validated."); Unfortunately, the claimExact() function does not have the same assertion allowing the amount for a user to be lowered to zero.
4. validate(): Once the airdrop users balance is zeroed, the can then call validate again, resetting their balance and allowing them to transfer twice the amount initially registered.
5. claimExact(): This is what allows them to withdraw the funds to their allocated address

Brownie test

def test_double_claim(airdrop, chain, token, vesting):
    chain.sleep(ONE_WEEK//2)
    airdrop.validate({"from":AIRDROP_ADDR})
    init_supply =  20 * 10 ** 18
    assert(token.balanceOf(AIRDROP_ADDR) == 0)

    fraction = 10**18 * (AIRDROP_EXPECTED * 10 ** 18) / init_supply
    airdrop_user = airdrop.airdrop(AIRDROP_ADDR)
    assert airdrop_user["fraction"] - fraction == 0
    
    avail_supp = airdrop.available_supply()
    exact_amount = avail_supp * (airdrop_user["fraction"]) / 10 ** 18
    assert exact_amount == avail_supp * (airdrop_user["fraction"]) / 10 ** 18

    airdrop.claimExact(exact_amount - 100, {"from": AIRDROP_ADDR})
    balance = token.balanceOf(AIRDROP_ADDR)
    assert balance == (3 * avail_supp * airdrop_user["fraction"]) / (10**19)
    
    airdrop_user = airdrop.airdrop(AIRDROP_ADDR)
    assert airdrop_user["amount"] == 0

    airdrop.validate({"from": AIRDROP_ADDR})
    airdrop.claimExact(exact_amount - 100, {"from": AIRDROP_ADDR})
    balance = token.balanceOf(AIRDROP_ADDR)
    assert balance ==  2* (3 * avail_supp * airdrop_user["fraction"]) / (10**19) # This assert shows that the user has managed to withdraw more funds than initially allocated
    
    airdrop_user = airdrop.airdrop(AIRDROP_ADDR)
    assert airdrop_user["amount"] == 0

NOTE: For test to work supply was hard coded to uint256 private airdrop_supply = 20 * 10 ** 18; and _available_supply() was changed to use a fixed value. This does not impact the test functionality, but just aids in making the POC easier create without having to manage a large number of EPOCH intervals

function _available_supply() private view returns(uint256) {
        // assert(block.timestamp - startEpochTime <= RATE_TIME);
        // return startEpochSupply + (block.timestamp - startEpochTime) * rate;
        return airdrop_supply;
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Several mitigation strategies exist

1. add the same assertion to claimExact if this is the intended functionality expected (ie... user shouldn't have an amount of zero)
2. The above method seems like a misconfiguration as user should be theoretically able to drain their balance... Therefore it might be worth adding a maxClaimable to the airdrop struct and checking in validate() that the require(airdrop[msg.sender].claimed != airdrop[msg.sender].maxClaimable, "Already claimed.");

Handle

elprofesor

Vulnerability details

Impact

Swap.sol inherits ownerpausable which inherits from Open Zep Ownable. This ownable contract allows for the transfer of ownership without validating that own address is a valid address in control of some expected recipient. If this function is used incorrectly or by accident, the admin user may be lost or set to a malicious account.

Recommended Mitigation Steps

Implement a transfer-accept ownership pattern in Swap.sol contract. This allows an owner to accept the transfer insuring that the account is controlled by a valid user.