Wrong element is popped from reserveTokens list when removing an asset

[code423n4]

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/eb4d4a201b3a75dd4bddc74a34e9c42c71d0d12f/contracts/core/RdpxV2Core.sol#L283-L287

Vulnerability details

Impact

When an asset is removed from token reserves, a wrong element is removed. This will cause the list of reserve token symbols to not match the actual assets, which will cause next removals to perform incorrect index assignment and provide incorrect data for external reads.

Proof of Concept

The function RdpxV2Core.removeAssetFromtokenReserves pops the last element from reserveTokens without assigning the last element (which does not have to be the element that is removed) to a prior index:

reservesIndex[reserveTokens[reserveTokens.length - 1]] = index;

reserveAsset[index] = reserveAsset[reserveAsset.length - 1];

// remove the last element
reserveAsset.pop();
reserveTokens.pop();

Because reserveTokens is not changed before the pop, a wrong symbol is removed.

Tools Used

Manual Review

Recommended Mitigation Steps

Add the following line to removeAssetFromtokenReserves

reserveTokens[index] = reserveTokens[reserveTokens.length - 1]

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as duplicate of #33

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[liveactionllama]

Per discussion with judge, adding grade labels on their behalf.