Wrong implementation of twAML in function TapiocaOptionBroker.exitPosition()

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol#L314-L322

Vulnerability details

Impact

Incorrect accounting for pool.cumulative can lead to incorrect voting power calculation in the broker contract.

Proof of Concept

The function TapiocaOptionBroker.exitPosition() is utilized to exit a twAML (twTAP) participation.

/// link = https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/options/TapiocaOptionBroker.sol#L314-L322

function exitPosition(uint256 _oTAPTokenID) external {
    ... 

    if (participation.divergenceForce) {
        if (pool.cumulative > pool.averageMagnitude) {
            pool.cumulative -= pool.averageMagnitude;
        } else {
            pool.cumulative = 0;
        }
    } else {
        pool.cumulative += pool.averageMagnitude;
    }

    ...
}

Note that the participation entry tracks the average magnitude as it was at the time the participant entered. So, to exit the twAML, we need to reverse the participation process. In other words, if the position has voting power and is divergenceForce, the correct exit process should be:

if (position.divergenceForce) {
    if (pool.cumulative > participation.averageMagnitude) {
        pool.cumulative -= participation.averageMagnitude;
    } else {
        pool.cumulative = 0;
    }
} else {
    pool.cumulative += participation.averageMagnitude;
}

However, in the function TapiocaOptionBroker.exitPosition(), the pool.cumulative is updated with pool.averageMagnitude instead of participation.averageMagnitude, leading to an incorrect implementation.

To consolidate the issue, we can examine the update of exit process in twTAP.sol in this link:

• https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/governance/twTAP.sol#L543-L551

Tools Used

Manual review

Recommended Mitigation Steps

Modifying the update of pool.cumulative as follows:

if (position.divergenceForce) {
    if (pool.cumulative > participation.averageMagnitude) {
        pool.cumulative -= participation.averageMagnitude;
    } else {
        pool.cumulative = 0;
    }
} else {
    pool.cumulative += participation.averageMagnitude;
}

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora (sponsor) confirmed

[0xRektora]

Dup #1623

[c4-judge]

dmvt marked the issue as duplicate of #1623

[c4-judge]

dmvt marked the issue as satisfactory