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
Rank: 17/69
Findings: 2
Award: $311.49
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: AkshaySrivastav
Also found by: 0xjuicer, Bauer, Tajobin, adriro, csanuragjain, gzeon, immeas, rbserver
275.2478 USDC - $275.25
https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/cash/kyc/KYCRegistry.sol#L79
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
User KYC is approved and he is provided with a signature signed by an address with the role kycGroupRoles[kycRequirementGroup]
User claims his KYC approved status using addKYCAddressViaSignature
function which marks him kycState[kycRequirementGroup][user] = true
One of KYC group role realizes that User KYC was not actually proper and decides to remove his KYC status using removeKYCAddresses
function
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); }
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
🌟 Selected for report: CodingNameKiki
Also found by: 0x1f8b, 0x52, 0x5rings, 0xAgro, 0xSmartContract, 0xcm, 0xkato, 2997ms, Aymen0909, BClabs, BPZ, BRONZEDISC, Bauer, Bnke0x0, Deekshith99, IllIllI, Josiah, Kaysoft, RaymondFam, Rolezn, SaeedAlipoor01988, Tajobin, Udsen, Viktor_Cortess, adriro, arialblack14, betweenETHlines, btk, chaduke, chrisdior4, cryptphi, csanuragjain, cygaar, defsec, descharre, erictee, gzeon, hansfriese, horsefacts, joestakey, koxuan, lukris02, luxartvinsec, nicobevi, oyc_109, pavankv, peanuts, rbserver, scokaf, shark, tnevler, tsvetanovv, zaskoh
36.2377 USDC - $36.24
https://github.com/code-423n4/2023-01-ondo/blob/main/contracts/lending/OndoPriceOracle.sol#L64
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
setPrice
functionfunction setPrice(address fToken, uint256 price) external override onlyOwner { uint256 oldPrice = fTokenToUnderlyingPrice[fToken]; fTokenToUnderlyingPrice[fToken] = price; emit UnderlyingPriceSet(fToken, oldPrice, price); }
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 expiresfunction 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