Minting is possible even after Maturity has reached

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-swivel/blob/main/Creator/ZcToken.sol#L147

Vulnerability details

Impact

The contract is missing check to see if the market maturity has already reached before performing minting operation. Thus tokens could be minted even for a matured market

Proof of Concept

1. User A calls splitUnderlying at Swivel.sol#L578 to deposit underlying and in the process split it into/mint

2. This internally calls mintZcTokenAddingNotional at MarketPlace.sol#L103 which subsequently calls ZcToken(mkt.zcTokenAddr).mint(t, a) for performing the mint

3. Below is the mint function at ZcToken.sol#L147

function mint(address t, uint256 a) external onlyAdmin(address(redeemer)) returns (bool) {
        _mint(t, a);
        return true;
    }

4. As we can see this is directly minting amount a without checking whether maturity has already reached. This means minting will happen for a matured market as well

Recommended Mitigation Steps

Kindly revise mint function as below

function mint(address t, uint256 a) external onlyAdmin(address(redeemer)) returns (bool) {
require(block.timestamp < maturity , "matured");
        _mint(t, a);
        return true;
    }

[JTraversa]

I think that the remediation is a nice suggestion but an attack vector needs to be identified? What are the negative externalities if tokens are minted post maturity?

[bghughes]

I think that the remediation is a nice suggestion but an attack vector needs to be identified? What are the negative externalities if tokens are minted post maturity?

I believe this is a good issue given that the user has the ability to mint new tokens at a maturity that the protocol does not expect. Moreover, defined in the audit's Areas of Concern:

3. Ensuring maturity is handled properly across the Marketplace, VaultTracker, and ZcToken.

[JTraversa]

Is "That the protocol does not expect" really a valid attack?

If the tokens do nothing post maturity, why would we add gas costs / what are the negative externalities of minting post maturity?

IMO "Handled Properly" doesn't mean "redundantly cover every single potential interaction regardless of its effects", it means do we actually do what our protocol says it does.

So im hoping to get a suggested attack vector, otherwise I would have to assume our current implementation is a valid gas optimization rather than a suboptimal implementation needing amelioration?

[robrobbins]

addressed: https://github.com/Swivel-Finance/gost/pull/417

[JTraversa]

I'd mention that we addressed this separate from this report and as QA alongside our integration of EIP-5095. I'd still ask that the judge either move this to QA, or the judge or warden provide a valid attack vector.

[0xean]

Downgrading to QA due to lack of clear path shown by warden that would lead to a loss of funds or leak of value.

[0xean]

Grouping with #62 the wardens QA report