code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

`SystemContract#setGasPrice()` could cause frequent inability to execute transactions #375

Closed c4-bot-4 closed 10 months ago

c4-bot-4 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/zevm/SystemContract.sol#L118-L127

Vulnerability details

Impact

The SystemContract's approach to updating the gas price for different chains is flawed, cause where as the Fungible module periodically sets new prices, this would not accurately reflect real-time/real-world gas prices. This could lead to transaction failures during periods of network congestion or abrupt gas price changes.

Proof of Concept

The setGasPrice function allows the Fungible module to update the gas price for specific chains. However, this method doesn't guarantee real-time price accuracy and is susceptible to network conditions and potential manipulation.

Take a look at SystemContract.sol#L118-L127

function setGasPrice(uint256 chainID, uint256 price) external {
    if (msg.sender != FUNGIBLE_MODULE_ADDRESS) revert CallerIsNotFungibleModule();
    gasPriceByChainId[chainID] = price;
    emit SetGasPrice(chainID, price);
}

Now since there is always a need for the Fungible module to periodically update the price, an attacker could either front run a transaction to update the price if the difference is going to be massive, additionally this doesn't actually give the best approach, cause the gas price would never relate to the current world price and as such, whenever say a particular blockchain network gets congested and the price for gas abruptly increases, this would be a problem as the price has not being increased and transactions would keep on failing, since protocol asks for lower gas from users.

Tool Used

Manual Review

Recommended Mitigation Steps

To ensure accurate, real-time gas pricing, integrate price oracles that provide current network gas prices. This would align the contract's gas prices with actual market conditions, reducing the likelihood of transaction failures due to underpriced gas, or have a similar approach like that of layerZero's estimateFees().

Assessed type

Context

DadeKuma commented 11 months ago

QA at best

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

c4-judge commented 11 months ago

0xean changed the severity to QA (Quality Assurance)

c4-judge commented 11 months ago

0xean marked the issue as grade-b