Doesn't check whether maturity is not in the past

[code423n4]

Lines of code

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashBase.sol#L38-L39 https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/lib/DateTime.sol#L41-L49

Vulnerability details

Impact

Maturity in wfCashBase.initialize() should be not past the max market index or in the past. However DateTime.isValidMaturity() only checks whether maturity is past the max market index.

Proof of Concept

The comment indicate that the maturity should be not past the max market index or in the past

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/wfCashBase.sol#L38-L39

        // Ensure that the maturity is not past the max market index, also ensure that the maturity
        // is not in the past. This statement will allow idiosyncratic (non-tradable) fCash assets.
        require(
            DateTime.isValidMaturity(cashGroup.maxMarketIndex, maturity, block.timestamp),
            "Invalid maturity"
        );

But DateTime.isValidMaturity only check whether maturity is past the max market index

https://github.com/code-423n4/2022-06-notional-coop/blob/main/notional-wrapped-fcash/contracts/lib/DateTime.sol#L41-L49

    function isValidMaturity(
        uint256 maxMarketIndex,
        uint256 maturity,
        uint256 blockTime
    ) internal pure returns (bool) {
        uint256 tRef = DateTime.getReferenceTime(blockTime);
        uint256 maxMaturity = tRef + DateTime.getTradedMarket(maxMarketIndex);
        // Cannot trade past max maturity
        if (maturity > maxMaturity) return false;

        // prettier-ignore
        (/* */, bool isValid) = DateTime.getBitNumFromMaturity(blockTime, maturity);
        return isValid;
    }

Tools Used

None

Recommended Mitigation Steps

Add a check for invalid maturity in the past

if (maturity < tRef) return false;

[jeffywu]

Can confirm that this check is worthwhile, however, I don't see how this can result in any loss of funds or denial of service. Matured fCash cannot be minted so the wrapper would be useless.

[gzeoneth]

Consider with #152