`PoolCommitter.sol#._uncommit()` Tokens are not returned to the commit owner

[code423n4]

Handle

WatchPug

Vulnerability details

https://github.com/tracer-protocol/perpetual-pools-contracts/blob/646360b0549962352fe0c3f5b214ff8b5f73ba51/contracts/implementation/PoolCommitter.sol#L222-L231

if (_commit.commitType == CommitType.LongMint || _commit.commitType == CommitType.ShortMint) {
    // minting: return quote tokens to the commit owner
    ILeveragedPool(leveragedPool).quoteTokenTransfer(msg.sender, _commit.amount);
} else if (_commit.commitType == CommitType.LongBurn) {
    // long burning: return long pool tokens to commit owner
    ILeveragedPool(leveragedPool).mintTokens(0, _commit.amount, msg.sender);
} else if (_commit.commitType == CommitType.ShortBurn) {
    // short burning: return short pool tokens to the commit owner
    ILeveragedPool(leveragedPool).mintTokens(1, _commit.amount, msg.sender);
}

As _uncommit() is called by executeAllCommitments, and the executeAllCommitments is called by LeveragedPool.poolUpkeep.

Therefore, in _uncommit(), the msg.sender will be the LeveragedPool.sol contract.

Per the comment, _uncommit() is expected to:

minting: return quote tokens to the commit owner long burning: return long pool tokens to commit owner short burning: return short pool tokens to the commit owner

However, the current implementation tokens will be returned to the LeveragedPool.sol contract instead of the commit owner.

Impact

Commit owners will lose fund when IPoolCommitter(address(this)).executeCommitment(_commit) fails.

Recommendation

Change to:

if (_commit.commitType == CommitType.LongMint || _commit.commitType == CommitType.ShortMint) {
    // minting: return quote tokens to the commit owner
    ILeveragedPool(leveragedPool).quoteTokenTransfer(_commit.owner, _commit.amount);
} else if (_commit.commitType == CommitType.LongBurn) {
    // long burning: return long pool tokens to commit owner
    ILeveragedPool(leveragedPool).mintTokens(0, _commit.amount, _commit.owner);
} else if (_commit.commitType == CommitType.ShortBurn) {
    // short burning: return short pool tokens to the commit owner
    ILeveragedPool(leveragedPool).mintTokens(1, _commit.amount, _commit.owner);
}

[kumar-ish]

Duplicate of #19