Infinity NFT Marketplace contest - dipp's results

Lines of code

InfinityExchange.sol#L300-L328 InfinityExchange.sol#L336-L364

Vulnerability details

Impact

When using InfinityExchange.sol:takeMultipleOneOrders or InfinityExchange.sol:takeOrders, if currency == address(0) and a user has a msg.value > totalPrice the overspent ETH is left in the contract and not sent back to the user.

Proof of Concept

InfinityExchange.sol#L326

InfinityExchange.sol#L362

Recommended Mitigation Steps

Consider changing the msg.value >= totalPrice on line 326 and line 362 to msg.value == totalPrice or sending the difference back to the function user.

Lines of code

InfinityStaker.sol#L116-L131 InfinityStaker.sol#L290-L325

Vulnerability details

Impact

In InfinityStaker.sol, when calling the unstake function _updateUserStakedAmounts is called to readjust the user staked amounts. If a user stakes an amount in a duration and has an already vested amont in a higher duration, the user might lose the not yet vested stake in the lower duration.

This is due to the _updateUserStakedAmounts function setting the userStakedAmount on all durations to 0 if its vesting amount is 0 without checking if the duration has a non-zero staked amount.

Proof of Concept

1. A user has 1000 tokens staked in the TWELVE_MONTH duration which can be vested.

2. Before unstaking the vested TWELVE_MONTH stake, the user then stakes an additional 2000 tokens into the THREE_MONTH duration.

3. The user then calls unstake to unstake the 1000 tokens in the TWELVE_MONTH duration. (Before the THREE_MONTH stake can be vested)

4. The _updateUserStakedAmounts will then check if the unstake amount is more than the amount which can be vested for each duration.

if (amount > vestedThreeMonths) {
        userstakedAmounts[user][Duration.THREE_MONTHS].amount = 0;
        userstakedAmounts[user][Duration.THREE_MONTHS].timestamp = 0;
        amount = amount - vestedThreeMonths;

5. Since vestedThreeMonths is 0, the THREE_MONTH stake amount is set to 0 and eventually the TWELVE_MONTH stake is decreased by 1000.

6. The user receives 1000 tokens and loses the 2000 tokens staked in THREE_MONTHS.

Recommended Mitigation Steps

The userStakedAmount should only decrease if the vested amount > 0. Consider adding an additional check for each if statement in the _updateUserStakedAmounts to check if the vested amount for that duration is more than 0 and to only reset the duration's stake if this is true.