Uninitialized Variable in _getRequiredCollateralSingleLegPartner Function May Lead to Incorrect Collateral Calculations

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-06-panoptic/blob/main/contracts/CollateralTracker.sol#L1445

Vulnerability details

Impact

The _getRequiredCollateralSingleLegPartner function in the CollateralTracker contract has a logic path where the required variable may remain unassigned. Specifically, if isLong != tokenId.isLong(partnerIndex) is true and isLong == 0, the function does not assign a value to required. This can lead to uninitialized variables being used, causing unexpected behavior or vulnerabilities in the contract. Uninitialized variables can result in incorrect collateral calculations, potentially affecting the security and stability of the entire collateral management system.

Proof of Concept

Line of Code:

if (isLong != tokenId.isLong(partnerIndex)) {
    if (isLong == 1) {
        required = _computeSpread(
            tokenId,
            positionSize,
            index,
            partnerIndex,
            poolUtilization
        );
    }
} else {
    required = _computeStrangle(tokenId, index, positionSize, atTick, poolUtilization);
}

Proof:

Here's a scenario illustrating the issue:

1. isLong is 0.
2. tokenId.isLong(partnerIndex) is 1.
3. The condition if (isLong != tokenId.isLong(partnerIndex)) is true.
4. Since isLong == 0, the inner if (isLong == 1) is false, leading to no assignment for required.

This results in required being uninitialized, which can cause erroneous behavior when this variable is later used.

Tools Used

Manual

Recommended Mitigation Steps

Ensure that all code paths in _getRequiredCollateralSingleLegPartner assign a value to the required variable.

function _getRequiredCollateralSingleLegPartner(
    TokenId tokenId,
    uint256 index,
    uint128 positionSize,
    int24 atTick,
    uint128 poolUtilization
) internal view returns (uint256 required) {
    uint256 partnerIndex = tokenId.riskPartner(index);
    uint256 isLong = tokenId.isLong(index);

    if (isLong != tokenId.isLong(partnerIndex)) {
        if (isLong == 1) {
            required = _computeSpread(
                tokenId,
                positionSize,
                index,
                partnerIndex,
                poolUtilization
            );
        } else {
            // Handle the case where isLong == 0
            required = _computeSpread(
                tokenId,
                positionSize,
                partnerIndex,
                index,
                poolUtilization
            );
        }
    } else {
        required = _computeStrangle(tokenId, index, positionSize, atTick, poolUtilization);
    }
}

This ensures that the required variable is always assigned a value, preventing uninitialized variables from causing issues in the contract.

Assessed type

Context

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[Picodes]

uninitialized variables are 0