code-423n4 / 2022-06-connext-findings

1 stars 0 forks source link

reimburseRelayFee calculation doesn't check for 0 denominator when oracle is set #189

Closed code423n4 closed 1 year ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-connext/blob/b4532655071566b33c41eac46e75be29b4a381ed/contracts/contracts/core/connext/helpers/SponsorVault.sol#L234-L263

Vulnerability details

Impact

When oracle is present contract doesn't properly handle den = 0

Proof of Concept

If gasTokenOracle.getRate(_originDomain) in L244 returns 0 then sponsoredFee in the next line will error

Tools Used

Recommended Mitigation Steps

sponsoredFee is calculated later in the functions anyways at L253 so remove sponsoredFee at L246

jakekidd commented 2 years ago

Why would the gas token oracle return 0 for the rate? The gas token oracle address's assignment is controlled by the Owner. Whatever contract is set for this, we can simply ensure 0 is not a possible return value.

jakekidd commented 2 years ago

This seems like a QA issue to me - might be worth handling. The mitigation step also contains a gas optimization, so I'm confirming this as something that should be attended to.

LayneHaber commented 2 years ago

This won't cause a loss or leak of funds, but the sponsor vault will not work as intended. While the owner should check this before assigning an oracle, it's worth adding these sanity checks regardless, especially because there are a multitude of reasons the oracle could return 0

0xleastwood commented 1 year ago

Probably worth handling but the likelihood of this is extremely low. It assumes a faulty oracle contract.

0xleastwood commented 1 year ago

Merging with #194.