Tapioca DAO - dontonka's results

Lines of code

https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBox.sol#L196-L215 https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBox.sol#L49

Vulnerability details

Impact

The function depositETHAsset allows the caller to provide Ether, but does not refund the amount in excess of what's required, leaving funds stranded in the contract and the excess sent become essentially a loss for the user.

Proof of Concept

Add this test in YieldBox.ts and run it.

    describe("depositETH", () => {
        //...

        // NEW TEST POC
        it.only("handles deposit of ETH (msg.values > amount), ", async function () {
            // send an higher ETH amount then the amount specified in parameter
            await yieldBox.depositETH(ethStrategy.address, Alice, 1000, { value: 10000 })

            // value held for the user is 1000, not 10000, so 9000 become essentially a loss for the user
            expect(await yieldBox.balanceOf(Alice, 2)).equals(1000_00000000)
        })

        //...
    })

Tools Used

Test suite, VC Code

Recommended Mitigation Steps

Minimal change would be to ensure amount and msg.value match with a require statement.

diff --git a/contracts/YieldBox.sol b/contracts/YieldBox.sol
index 4cf3d71..f7023d8 100644
--- a/contracts/YieldBox.sol
+++ b/contracts/YieldBox.sol
@@ -196,6 +196,7 @@ contract YieldBox is YieldBoxPermit, BoringBatchable, NativeTokenFactory, ERC721
     function depositETHAsset(uint256 assetId, address to, uint256 amount) public payable returns (uint256 amountOut, uint256 shareOut) {
         // Checks
         Asset storage asset = assets[assetId];
+        require(amount == msg.value, "!diff amount");
         require(asset.tokenType == TokenType.ERC20 && asset.contractAddress == address(wrappedNative), "YieldBox: not wrappedNative");

         // Effects

Assessed type

Payable