hats-finance / Inverter-Network-0xe47e52c4fea05e555920f1dcdcc6fb8eca103eeb

Fork of the Inverter Smart Contracts Repository
GNU Lesser General Public License v3.0
0 stars 3 forks source link

`init()` not using precise year value to validate the `epochLength` #90

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0xb6f36dade49dda07d08860f40e8a2ff4a120d130add81992addfe9718c463b86 Severity: low

Description: Description\

LM_PC_RecurringPayments_v1.sol has implemented init() function as:

    function init(
        IOrchestrator_v1 orchestrator_,
        Metadata memory metadata,
        bytes memory configData
    ) external override(Module_v1) initializer {
        __Module_init(orchestrator_, metadata);
        // Set empty list of RecurringPayment
        _paymentList.init();

        epochLength = abi.decode(configData, (uint));

        // revert if not at least 1 week and at most a year
@>      if (epochLength < 1 weeks || epochLength > 52 weeks) {
            revert Module__LM_PC_RecurringPayments__InvalidEpochLength();
        }
    }

epochLength is being validated as not less than a WEEK and not more than a YEAR. For validation with YEAR, function has used 52 weeks which is however not precise for a year value.

The YEAR is multiplication of DAY with 365 days and it is true in most of cases, the exact YEAR would be DAY multiply with 365.25(which is the average number of days in the Julian calendar) for a more precise calculation. This also covers the leap year which comes every four year. Taking 365.25 days would be very price considering the use of epochLength in contracts functions such as: getEpochLength(), getEpochFromTimestamp(), getCurrentEpoch(), etc.

Therefore, consider 3,15,57,600 seconds instead of 52 weeks.

Recommendation to fix\

Consider below changes in init() function, epochLength is not validated in terms of seconds instead of weeks.

1 Week as 604800 seconds and 31557600 as 365.25 days.

    function init(
        IOrchestrator_v1 orchestrator_,
        Metadata memory metadata,
        bytes memory configData
    ) external override(Module_v1) initializer {
        __Module_init(orchestrator_, metadata);
        // Set empty list of RecurringPayment
        _paymentList.init();

        epochLength = abi.decode(configData, (uint));

        // revert if not at least 1 week and at most a year
-        if (epochLength < 1 weeks || epochLength > 52 weeks) {
+        if (epochLength < 604800 || epochLength > 31557600) {
            revert Module__LM_PC_RecurringPayments__InvalidEpochLength();
        }
    }
PlamenTSV commented 4 months ago

Pretty sure it is intended. Method used is called Stated Rate Method which rounds down the lost quarter back to 365. Will leave it unmarked for now