Unrestricted Shares Transferability

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L319-L353

Vulnerability details

Impact

The contract does not seem to limit shares transferrability. The lack of restrictions makes it possible for users to reduce their collateral by sending shares to other addresses. This may lead to situations when some positions are under-collateralized which can pose a liquidity risk to the protocol.

Proof of Concept

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L319-L353

function transfer(
    address recipient,
    uint256 amount
) public override(ERC20Minimal) returns (bool) {
    // make sure the caller does not have any open option positions
    if (s_panopticPool.numberOfPositions(msg.sender) != 0) revert Errors.PositionCountNotZero();
    return ERC20Minimal.transfer(recipient, amount);
}
function transferFrom(
    address from,
    address to,
    uint256 amount
) public override(ERC20Minimal) returns (bool) {
    // make sure the caller does not have any open option positions
    if (s_panopticPool.numberOfPositions(from) != 0) revert Errors.PositionCountNotZero();
    return ERC20Minimal.transferFrom(from, to, amount);
}

The transfer function checks if the senders have open positions in the protocol. However, if the sender has closed their positions before executing the transfers, they can still drain their balance.

Tools Used

Manual review

Recommended Mitigation Steps

To properly manage the risk of under-collateralization, it would be advisable to have stricter rules or restrictions around the transfer of shares. This might include implementing checks to prevent the transfer of shares from reducing a user's collateral below the required minimum for their open positions.

Assessed type

Invalid Validation

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

But then if there are no open positions it can't go undercollat?