`RdpxV2Core.sol`: `addAssetTotokenReserves` does not check conflicting asset symbols

[code423n4]

Lines of code

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

Vulnerability details

Impact

The function addAssetTotokenReserves checks if an asset is already present before adding them to the array.

for (uint256 i = 1; i < reserveAsset.length; i++) {
    require(
    reserveAsset[i].tokenAddress != _asset,
    "RdpxV2Core: asset already exists"
    );
}

This is because it can break the index mappings if an array address gets repeated. However there is no such check for reserveTokens array, which store the token symbols.

There are multiple tokens with the same symbols. A common example is token upgrades, where the token contracts are updated but the symbol/name is kept the same. If for some reason a user tries to add a new token which has the symbol of an existing token in the array, the mappings will break down.

The require statement will pass, since the addresses are different. However at the end, the conflicting symbol will be added to the reserveTokens array.

reserveAsset.push(asset);
reserveTokens.push(_assetSymbol);
reservesIndex[_assetSymbol] = reserveAsset.length - 1;

The main issue comes with the last line, where the index mapping is updated. Since the symbol is used as the key, it wil overwrite the older token with the same symbol. Thus the token overwritten will be inaccessible.

Proof of Concept

The code never checks for the existing values in reserveTokens array.

Tools Used

Manual Review

Recommended Mitigation Steps

Add a check that the symbol is not already used.

require(reservesIndex[_assetSymbol] = 0);

Assessed type

Error

[c4-pre-sort]

bytes032 marked the issue as duplicate of #1193

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

bytes032 marked the issue as low quality report

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)