ravikiran.web3
medium
In the addCollateral function, index passed as parameter decides on whether is an update of old collateral or a new collateral. The logic does not perform enough checks before updating the storage variable. The only defence is the modifier that allows only control user to call this function. This is very centralized and assuming there is no mistake while updating.
AddCollateral: The overwite of collateral based on index alone is risky. Instead, before updating the collateral object, compare the address of the param to match the address in the storage at the index location.
Example: In position 2, there could have be a collateral for DAI, but if i pass a new collateral for WETH on position 2, current logic will accept the value and override it since it just checks for index to exist. Also, ignores the earlier collateral and just overrides it.
Since collateral is critical in the rebalance and minting of the USSD, accurate management of collateral is important. In correct collateral will lead to incorrect rebalancing.
https://github.com/sherlock-audit/2023-05-USSD/blob/main/ussd-contracts/contracts/USSD.sol#L84-L108
if (index < collateral.length) { collateral[index] = newCollateral; // for editing } else { collateral.push(newCollateral); // for adding new collateral } }
Manual Review
modify the code to compare the storage collateral address to be same as incoming param.