howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-munchables/blob/94cf468aaabf526b7a8319f7eba34014ccebe7b9/src/managers/LandManager.sol#L283-L286

Vulnerability details

Impact

When calculating schnibblesTotal in LandManager::_farmPlots, the bonus is applied as a percentage of the accumulated schnibbles based on the base rate. However, instead of dividing by 100 just the amount corresponding to the bonus, the entire sum is divided by 100:

schnibblesTotal = uint256(
    (int256(schnibblesTotal) +
        (int256(schnibblesTotal) * finalBonus)) / 100 
);

This results in a smaller amount than expected for both the renter and the landlord, effectively reducing their rewards unfairly.

Proof of Concept

In the example below I test a case in which Bob stakes for one second in Alice's plot. The paramaters are as follows:

BASE_SCHNIBBLE_RATE: 1e15
latestTaxRate: 45e16
finalBonus: 30
timestamp - _toiler.lastToilDate: 1

The expected total amount is: $11e15+11e1530/100=1.3e15$, however the actual result is: $(11e15+11e1530)/100=0.31e15$. The amount Bob gets is proportionally calculated based on the tax rate. In this example it can be observed how the contract is stealing $0.5445e15$ schnibbles per second from Bob.

See PoC

You can place this in `unstakeMunchable.test.ts`. ```javascript it("Erroneous schnibbles calculation", async () => { await testContracts.landManagerProxy.contract.write.updateTaxRate([45e16], { account: alice, }); const txHash = await testContracts.munchNFT.contract.write.approve( [testContracts.landManagerProxy.contract.address, 1], { account: bob } ); await assertTxSuccess({ txHash }); const { request: stakeRequest } = await testContracts.landManagerProxy.contract.simulate.stakeMunchable( [alice, 1, 0], { account: bob, } ); const stakeMunchableTxHash = await testClient.writeContract(stakeRequest); const receiptStake = await assertTxSuccess({ txHash: stakeMunchableTxHash }); // Wait for the staking transaction to be mined const txBlockStake = await testClient.getBlock({ blockHash: receiptStake.blockHash, }); console.log("Staked at: ", Number(txBlockStake.timestamp)); const immutableAttributesRead = await testContracts.nftAttributesManagerV1.contract.read.getImmutableAttributes( [1n], { account: bob, } ); console.log("Rarity: ", immutableAttributesRead.rarity); console.log("Realm: ", immutableAttributesRead.realm); const playerSettings = await testContracts.accountManagerProxy.contract.read.getPlayer([alice]); console.log("Snuggery realm: ", playerSettings[1].snuggeryRealm); // bonus = 30 // BASE_SCHNIBBLE_RATE = 1e15 // schnibblesTotal = 31*1*1e15/100 = 31e13 // correct schnibblesTotal = 1e15 + 30*1e15/100 = 1.3e15 const schnibblesbefore = await testContracts.accountManagerProxy.contract.read.getPlayer([bob]); console.log("Unfed Schnibbles Before: ", schnibblesbefore[1].unfedSchnibbles); const { request: unstakeRequest } = await testContracts.landManagerProxy.contract.simulate.unstakeMunchable( [1], { account: bob, } ); const unstakeHash = await testClient.writeContract(unstakeRequest); const unstakeHashReceipt = await assertTxSuccess({ txHash: unstakeHash }); // Wait for the unstaking transaction to be mined const txBlockUnstake = await testClient.getBlock({ blockHash: unstakeHashReceipt.blockHash, }); console.log("Unstaked at: ", Number(txBlockUnstake.timestamp)); const unstakePlayerSettings = await testContracts.accountManagerProxy.contract.read.getPlayer([bob]); console.log("Unfed Schnibbles After: ", unstakePlayerSettings[1].unfedSchnibbles); // bob schnibbles = schinbblesTotal*(1-latestTaxRate) = schnibblesTotal*0.55 // ASIS: 31e13*0.55 = 0,1705e15 // correct: 1.3e15*0.55 = 0,715e15 const timeDifference = txBlockUnstake.timestamp - txBlockStake.timestamp; console.log("Time difference between stake and unstake: ", timeDifference); }); ```

Tools Used

Manual review

Recommended Mitigation Steps

Update to corrected formula:

schnibblesTotal = uint256(
    (int256(schnibblesTotal) +
        (int256(schnibblesTotal) * finalBonus) /
        100)
);

Assessed type

Math

c4-judge commented 3 months ago

alex-ppg marked the issue as satisfactory