Insufficient access control when setting the initial Feeds.

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/price_feed/PriceAggregator.sol#L37-L44

Vulnerability details

During initialization, the owner will call the setInitialFeeds() function and set price feeds. Note that, this transaction is called by an EOA address instead of the DAO contract. We should be very careful to avoid centralized risk.

Unfortunately, the lack of abundant access control in the setInitialFeeds() function may result in price manipulation. Currently, it only requires address(priceFeed1) == address(0). However, priceFeed1, priceFeed2, and priceFeed3 should be differential, to avoid price manipulation attacks.

Note that, even the Deployment script itself makes such a mistake. See here.

So it is very important to add access control that require(address(priceFeed1) != address(priceFeed2) && address(priceFeed2) != address(priceFeed3) && address(priceFeed1) != address(priceFeed3)).

Impact

Erroneous settings in the price aggregator may result in price manipulation attacks.

Assessed type

Access Control

[c4-judge]

Picodes marked the issue as duplicate of #970

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

Picodes marked the issue as grade-b