code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/libraries/DateTimeLib.sol#L55-L61 https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/vMaia.sol#L109-L110

Vulnerability details

Impact

DateTimeLib.isTuesday serves to determine whether or not the input timestamp is a Tuesday of a month. This can easily be bricked since there are 4 or 5 Tuesdays in every calendar month which allows users withdraw from their vMaia positions 4 to 5 times instead of just once a month.

Proof of Concept

The following code block on line 109 is the only place where DateTimeLib.isTuesday is called after ensuring _currentMonth != currentMonth:

https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/vMaia.sol#L102-L114

    function beforeWithdraw(uint256, uint256) internal override {
        /// @dev Check if unstake period has not ended yet, continue if it is the case.
        if (unstakePeriodEnd >= block.timestamp) return;

        uint256 _currentMonth = DateTimeLib.getMonth(block.timestamp);
        if (_currentMonth == currentMonth) revert UnstakePeriodNotLive();

109:        (bool isTuesday, uint256 _unstakePeriodStart) = DateTimeLib.isTuesday(block.timestamp);
        if (!isTuesday) revert UnstakePeriodNotLive();

        currentMonth = _currentMonth;
        unstakePeriodEnd = _unstakePeriodStart + 1 days;
    }

And. as can be seen in DateTimeLib.isTuesday, the function simply returns the weekday from the unix timestamp determining whether or not ((day + 3) % 7) + 1 == 2 or isTuesday is true to the caller. No where in the logic is seen as making sure it is the first week (or 7 days) of the month:

https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/libraries/DateTimeLib.sol#L53-L61

    /// @dev Returns the weekday from the unix timestamp.
    /// Monday: 1, Tuesday: 2, ....., Sunday: 7.
    function isTuesday(uint256 timestamp) internal pure returns (bool result, uint256 startOfDay) {
        unchecked {
            uint256 day = timestamp / 86400;
            startOfDay = day * 86400;
            result = ((day + 3) % 7) + 1 == 2;
        }
    }

Recommended Mitigation Steps

A simple solution to this problem could be to check that the day of the month is less than or equal to 7. This is because the first Tuesday of the month will always fall within the first seven days of the month.

To implement this, you might add another function to your DateTimeLib library that calculates the day of the month, and then use this function in the beforeWithdraw method.

Here is a rough outline of what this function could look like:

/// @dev Returns the day of the month from the unix timestamp.
function getDay(uint256 timestamp) internal pure returns (uint256 day) {
    uint256 epochDay = timestamp / 86400;

    /// @solidity memory-safe-assembly
    assembly {
        // ... similar calculations as in getMonth ...

        day := add(sub(doy, mul(5, mp)), 123)
    }
}

In beforeWithdraw:

/// ...other code...

(uint256 day, ) = DateTimeLib.getDay(block.timestamp);
if (day > 7) revert UnstakePeriodNotLive();

/// ...other code...

This implementation assumes that you have a similar calculation in getDay as you have in getMonth. This might require adjustments depending on your precise date algorithms and requirements.

On a side note for informational purpose, the comment denoted as:

https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/libraries/DateTimeLib.sol#L27-L29

    // "And on the seventh day God finished his work that he had done,
    // and he rested on the seventh day from all his work that he had done."
    // -- Genesis 2:2

is Biblically incorrect when implemented in DateTimeLib.isTuesday considering the first day of the week is Sunday instead of Monday:

https://github.com/code-423n4/2023-05-maia/blob/main/src/maia/libraries/DateTimeLib.sol#L54

    /// Monday: 1, Tuesday: 2, ....., Sunday: 7.

Assessed type

Math

c4-judge commented 1 year ago

trust1995 marked the issue as duplicate of #396