163.md

hansfriese

medium

The period for forex options can be larger unexpectedly

Summary

When a pending order is opened by a keeper, the option parameters are validated in BufferBinaryOptions.checkParams. At first, if the option type is forex, it validates the period using BufferBinaryOptions.isInCreationWindow. But there is a bug in isInCreationWindow. So the period can be larger than it should be.

Vulnerability Detail

        uint256 currentDay = ((currentTime / 86400) + 4) % 7;
        uint256 expirationDay = (((currentTime + period) / 86400) + 4) % 7;

        if (currentDay == expirationDay) {  // @audit - checked only days of the week

In isInCreationWindow, it gets the days of week of the current day and the expired day respectively, and checks if they are the same. But the forex trading is handled on Intraday, and the current time and expire time (current time + period) should be on same day. The check using days of week is valid only the period is smaller than 7 days. Actually the period is validated from config.maxPeriod(), and the default value of config.maxPeriod() is 1 day. But when the admin set this maxPeriod, it checks only the lower bound.

    function setMaxPeriod(uint32 value) external onlyOwner {
        require(
            value >= 1 minutes,
            "MaxPeriod needs to be greater than 1 minutes"
        );

So technically the maxPeriod can be any value. If maxPeriod and period is 7 days, current time and expire time are not in the same day, but the days of the weeks are the same, and the if statement will be passed.

Impact

The period for forex options can be larger than 1 day.

Code Snippet

https://github.com/sherlock-audit/2022-11-buffer/blob/main/contracts/contracts/core/BufferBinaryOptions.sol#L242-L245 https://github.com/sherlock-audit/2022-11-buffer/blob/main/contracts/contracts/core/OptionsConfig.sol#L67-L71

Tool used

Manual Review

Recommendation

Check if current day and expire day are the same exactly. Add upper bound validation to OptionsConfig.setMaxPeriod.