code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-08-dopex/blob/main/contracts/core/RdpxV2Core.sol#L270-L290

Vulnerability details

Impact

Removing work incorrectly. It clears reserveAsset and reserveTokens incorrectly.

Proof of Concept

  function removeAssetFromtokenReserves(
    string memory _assetSymbol
  ) external onlyRole(DEFAULT_ADMIN_ROLE) {
    uint256 index = reservesIndex[_assetSymbol];
    _validate(index != 0, 18);

    reservesIndex[_assetSymbol] = 0;

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

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

    reserveAsset.pop();
    reserveTokens.pop();

    emit LogAssetRemovedFromtokenReserves(_assetSymbol, index);
  }

This code won't work with case when item to remove is the last. Moreover this code might broke whole protocol because it doesn't remove element from reserveTokens Must be this line: reserveTokens[index] = reserveTokens[reserveTokens.length - 1];

Tools Used

Recommended Mitigation Steps

Fix code to:

Handle removing last element;
Update index modification for reserveTokens: reserveTokens[index] = reserveTokens[reserveTokens.length - 1];

Assessed type

Other

c4-pre-sort commented 1 year ago

bytes032 marked the issue as duplicate of #33

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

c4-judge commented 1 year ago

GalloDaSballo changed the severity to 2 (Med Risk)