code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-amphora/blob/daae020331404647c661ab534d20093c875483e1/core/solidity/contracts/periphery/oracles/ChainlinkTokenOracleRelay.sol#L44-L52

Vulnerability details

Impact

The ChainlinkTokenOracleRelay contract, as explicitly stated from comments in the code is designed to serve as an oracle for tokens that lack a direct USD trading pair but do possess a wETH/ETH pair. However the contract calculation of the token's price is entirely a different thing.

Proof of Concept

Take a look at the _get() function, which is designed to retrieve and return the current token price.

function _get() internal view returns (uint256 _value) {
    uint256 _aggregatorPrice = AGGREGATOR.peekValue();
    uint256 _baseAggregatorPrice = BASE_AGGREGATOR.peekValue();

    _value = (_aggregatorPrice * _baseAggregatorPrice) / 1e18;
}

Say two tokens A and B, using the A/ETH and B/ETH chainlink feeds, this function performs this calculation as (A/ETH) * (B/ETH) / 1e18, where A and B are prices obtained from two separate Chainlink oraclest his computation results in a token's value being represented in terms of AB/ETH^2, instead of a USD value.

The function obtains prices from two different Chainlink oracles and performs a multiplication and division operation to derive the price. However, the price calculation does not correctly yield a USD value but instead presents the result in AB/ETH^2.

Recommended Mitigation Steps

The price calculation for each token should instead be revised to (A/ETH) * (ETH/USD) & (B/ETH) * (ETH/USD) or something along these lines

Assessed type

Context

c4-pre-sort commented 1 year ago

minhquanym marked the issue as low quality report

minhquanym commented 1 year ago

Seems like the code did exactly what described in the recommendation

c4-judge commented 1 year ago

dmvt marked the issue as unsatisfactory: Invalid

code-423n4 / 2023-07-amphora-findings