code-423n4 / 2022-11-size-findings

1 stars 0 forks source link

Contracts will not working correctly after February 2106. Vesting will be locked forever if withdrawn after February 2106. #177

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L460 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L405 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/util/CommonTokenMath.sol#L47-L69 https://github.com/code-423n4/2022-11-size/blob/79aa9c01987e57a760521acecfe81b28eab3b313/src/SizeSealed.sol#L358-L387

Vulnerability details

Impact

Contracts will not working correctly after February 2106. Migration takes costs and is risky. You shouldn't pass on this work to future programmers. You should fix it in the first place. In case anything went wrong during migration, a big fund loss will occur.

Vesting will be locked forever if withdrawn after February 2106. Moreover, there isn't any migration logic, so this should be high risk as funds may be lost forever.

Proof of Concept

    function tokensAvailableForWithdrawal(uint256 auctionId, uint128 baseAmount)
        public
        view
        returns (uint128 tokensAvailable)
    {
        Auction storage a = idToAuction[auctionId];
        return CommonTokenMath.tokensAvailableAtTime(
            a.timings.vestingStartTimestamp,
            a.timings.vestingEndTimestamp,
            uint32(block.timestamp),
            a.timings.cliffPercent,
            baseAmount
        );
    }

block.timestamp is converted to uint32 which has a max value of 4294967295. 4294967295 is February 7, 2106. After February 7, 2106, uint32(block.timestamp) will be overflowed. Once it is overflowed, the logic of the contract will be broken.

    function withdraw(uint256 auctionId, uint256 bidIndex) external atState(idToAuction[auctionId], States.Finalized) {
        Auction storage a = idToAuction[auctionId];
        EncryptedBid storage b = a.bids[bidIndex];
        if (msg.sender != b.sender) {
            revert UnauthorizedCaller();
        }

        uint128 baseAmount = b.filledBaseAmount;
        if (baseAmount == 0) {
            revert InvalidState();
        }

        uint128 baseTokensAvailable = tokensAvailableForWithdrawal(auctionId, baseAmount);
        baseTokensAvailable = baseTokensAvailable - b.baseWithdrawn;

        b.baseWithdrawn += baseTokensAvailable;

        // Refund unfilled quoteAmount on first withdraw
        if (b.quoteAmount != 0) {
            uint256 quoteBought = FixedPointMathLib.mulDivDown(baseAmount, a.data.lowestQuote, a.data.lowestBase);
            uint256 refundedQuote = b.quoteAmount - quoteBought;
            b.quoteAmount = 0;

            SafeTransferLib.safeTransfer(ERC20(a.params.quoteToken), msg.sender, refundedQuote);
        }

        SafeTransferLib.safeTransfer(ERC20(a.params.baseToken), msg.sender, baseTokensAvailable);

        emit Withdrawal(auctionId, bidIndex, baseTokensAvailable, baseAmount - b.baseWithdrawn);
    }

Here, vesting will be locked forever if withdrawn after February 2106. Since tokensAvailableForWithdrawal round block.timestamp to a very small number.

Recommended Mitigation Steps

Use larger unit such as uint40 instead of uint32

trust1995 commented 2 years ago

🤣

c4-judge commented 2 years ago

0xean marked the issue as unsatisfactory: Overinflated severity