code-423n4 / 2024-02-hydradx-findings

1 stars 0 forks source link

In several places, errors are not handled #159

Closed c4-bot-5 closed 6 months ago

c4-bot-5 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/omnipool/math.rs#L450 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/omnipool/math.rs#L454 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/omnipool/math.rs#L468 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/stableswap/math.rs#L426 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/stableswap/math.rs#L450 https://github.com/code-423n4/2024-02-hydradx/blob/main/HydraDX-node/math/src/stableswap/math.rs#L705

Vulnerability details

Impact

Not handling errors properly in critical operations can lead to vulnerabilities, inaccuracies, or unexpected behavior.

Proof of Concept

In several places in the protocol, errors are not handled various mathematical operations:

omnipool/math.rs:

450: price_a.checked_mul(&price_b)
454: FixedU128::checked_from_rational(asset.reserve, asset.hub_reserve)
468: FixedU128::one().checked_sub(&p)?.checked_mul_int(asset.reserve)

stableswap/math.rs:

426: calculate_y_internal::<Y>(&xp, d, amplification)
450: calculate_y_internal::<Y>(&xp, d, amplification)
705: let (num, denom) = if let Some(v) = denom.checked_mul(p_diff) {

Handling errors explicitly helps prevent vulnerabilities and ensures the contract behaves as expected even in edge cases.

Tools Used

Visual Studio Code

Recommended Mitigation Steps

For all these operations always check the result of operations that can fail and handle errors or None cases appropriately.

Assessed type

Math

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

OpenCoreCH commented 6 months ago

No proof that this could lead to problems and if you follow some of these functions, the errors are typically handled at some point (because None is returned which would panic later on).

c4-judge commented 6 months ago

OpenCoreCH marked the issue as unsatisfactory: Insufficient proof