Ondo Finance contest - csanuragjain'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: 17/69

Findings: 2

Award: $311.49

QA:
grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: AkshaySrivastav

Also found by: 0xjuicer, Bauer, Tajobin, adriro, csanuragjain, gzeon, immeas, rbserver

Labels

bug
2 (Med Risk)
satisfactory
duplicate-187

Awards

275.2478 USDC - $275.25

External Links

Lines of code

https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/kyc/KYCRegistry.sol#L79

Vulnerability details

Impact

It is possible to remove the KYC status of any user by using the removeKYCAddresses function. This could be easily overridden by user using the addKYCAddressViaSignature function as shown below

Proof of Concept

  1. User KYC is approved and he is provided with a signature signed by an address with the role kycGroupRoles[kycRequirementGroup]

  2. User claims his KYC approved status using addKYCAddressViaSignature function which marks him kycState[kycRequirementGroup][user] = true

  3. One of KYC group role realizes that User KYC was not actually proper and decides to remove his KYC status using removeKYCAddresses function

  4. This sets kycState[kycRequirementGroup][user] = false

function removeKYCAddresses( uint256 kycRequirementGroup, address[] calldata addresses ) external onlyRole(kycGroupRoles[kycRequirementGroup]) { uint256 length = addresses.length; for (uint256 i = 0; i < length; i++) { kycState[kycRequirementGroup][addresses[i]] = false; } emit KYCAddressesRemoved(msg.sender, kycRequirementGroup, addresses); }
  1. But since the signature provided in Step 1 is still not expired so User simply recalls addKYCAddressViaSignature function to again set kycState[kycRequirementGroup][user] = true
function addKYCAddressViaSignature( uint256 kycRequirementGroup, address user, uint256 deadline, uint8 v, bytes32 r, bytes32 s ) external { .... kycState[kycRequirementGroup][user] = true; emit KYCAddressAddViaSignature( msg.sender, user, signer, kycRequirementGroup, deadline ); }

Mark the signature as used

#0 - c4-judge

2023-01-22T15:55:21Z

trust1995 marked the issue as duplicate of #187

#1 - c4-judge

2023-01-23T14:47:40Z

trust1995 marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

Once a fixed price has been set by owner, it never expires until owner explicitly sets it to 0. This could be risky where contract might be working with an obsolete price, if Owner failed to update price timely

Proof of Concept

  1. Owner sets fixed price P1 for fToken F using setPrice function
function setPrice(address fToken, uint256 price) external override onlyOwner { uint256 oldPrice = fTokenToUnderlyingPrice[fToken]; fTokenToUnderlyingPrice[fToken] = price; emit UnderlyingPriceSet(fToken, oldPrice, price); }
  1. After X days, if the getUnderlyingPrice is retrieved then price P1 will be returned for fToken F. This shows that fixed price which was set X days ago still works and never expires
function getUnderlyingPrice( address fToken ) external view override returns (uint256) { if (fTokenToUnderlyingPrice[fToken] != 0) { return fTokenToUnderlyingPrice[fToken]; } else { ... } }

Fixed price must expire after x seconds of addition. This will prevent contract from using obsolete prices

#0 - c4-judge

2023-01-23T14:42:46Z

trust1995 changed the severity to QA (Quality Assurance)

#1 - c4-judge

2023-01-23T14:42:52Z

trust1995 marked the issue as grade-b

#2 - c4-sponsor

2023-01-23T18:06:40Z

tom2o17 marked the issue as sponsor acknowledged

#3 - tom2o17

2023-01-23T18:07:40Z

Not super concerned with having the price expire. This manual setting of the oracle price is really only intended to be used wrt to CASH token

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