Ondo Finance contest - SaeedAlipoor01988's results

Institutional-Grade Finance. On-Chain. For Everyone.

General Information

Platform: Code4rena

Start Date: 11/01/2023

Pot Size: $60,500 USDC

Total HM: 6

Participants: 69

Period: 6 days

Judge: Trust

Total Solo HM: 2

Id: 204

League: ETH

Ondo Finance

Findings Distribution

Researcher Performance

Rank: 50/69

Findings: 1

Award: $36.24

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

In the below function https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L305

setter admin will set a new exchange rate for a certain epoch. But as you see, there is an if block, if rateDifference is more than maxDifferenceThisEpoch , we will set the exchange rate for the match epoch and call the pause function and emit the MintExchangeRateCheckFailed event.

Mint Exchange Rate Check Failed! else we will AGAIN set the exchange rate for the match epoch and emit the MintExchangeRateSet event. Why are we changing the exchange rate for the match epoch in both scenarios?

I think in first if block we should not make a change and we should remove epochToExchangeRate[epochToSet] = exchangeRate;

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

In below function https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L453

we are updating the recipient’s address to get the fee, but we don’t check whether this address is a zero address or not. The same is happening for the below function

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L466 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L807

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

There is not access limit for the below function https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L576 it’s not the problem?

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

In the below code, can we use <= https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L672

this is the comparison between the requested amount and minimumRedeemAmount. because this will save gas and I think there is no problem.

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

In the below function, https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/cash/CashManager.sol#L241 user comes and will claim for airdrop for a certain epoch, it’s not necessary to wait for the epoch to get end?

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

at the below function and line, we are getting the price of cToken Underlying. https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/CompoundLens.sol#L156

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/OndoPriceOracle.sol#L70

but we need to be sure that this price is updated recently and that we are not using the old price so we need to create if condition related to updatedAt for oracle price, if the condition is ok, then use the price. or we can use this condition directly in the price oracle contract, and before sending any price out! check the updatedAt of price.

please check below link: https://code4rena.com/reports/2022-08-olympus/#m-24-naz-m1-chainlinks-latestrounddata-might-return-stale-results

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

Not verified input. External/public function parameters should be validated to make sure the input values are correct. Otherwise if not given the right input it can mistakenly lead to a loss of user funds.

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/JumpRateModelV2.sol#L59 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/JumpRateModelV2.sol#L171 https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/JumpRateModelV2.sol#L83

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

0 address control should be done in these parts;

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/OndoPriceOracle.sol#L92

Recommended Mitigation Steps

Add code like this; if (cToken== address(0)) revert ADDRESS_ZERO();

////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

in the contract CCash.sol and line below, we use initialize function to initialize data for the new money market. but this function has no access limit and does not include a modifier to call only once.

https://github.com/code-423n4/2023-01-ondo/blob/f3426e5b6b4561e09460b2e6471eb694efdd6c70/contracts/lending/tokens/cCash/CCash.sol#L30

you can add initialized variable to the contract, and at the first call, after initialize data for the new money market, make initialized variable true.

#0 - c4-judge

2023-01-22T16:58:16Z

trust1995 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter