Platform: Code4rena
Start Date: 25/08/2022
Pot Size: $75,000 USDC
Total HM: 35
Participants: 147
Period: 7 days
Judge: 0xean
Total Solo HM: 15
Id: 156
League: ETH
Rank: 9/147
Findings: 4
Award: $2,324.37
🌟 Selected for report: 1
🚀 Solo Findings: 1
🌟 Selected for report: okkothejawa
347.2615 DAI - $347.26
https://github.com/code-423n4/2022-08-olympus/blob/277535739c465c75d37c33d706ab76365df2aade/src/modules/PRICE.sol#L165-L166 https://github.com/code-423n4/2022-08-olympus/blob/277535739c465c75d37c33d706ab76365df2aade/src/modules/PRICE.sol#L171-L172
Function OlympusPrice.getCurrentPrice returns ohm price in the reserve asset by retrieving prices of ohm and reserve asset from Chainlink. The issue is that the accepted timeframe of ohm price is 3 * uint256(observationFrequency):
if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) revert Price_BadFeed(address(_ohmEthPriceFeed));
but reserve asset is just uint256(observationFrequency):
if (updatedAt < block.timestamp - uint256(observationFrequency)) revert Price_BadFeed(address(_reserveEthPriceFeed));
This mismatch might lead to price inaccuracies and errors while the price of ohm is old comparing to the price of reserve asset. The accepted price timeframe should be exactly the same for both ohm and reserve asset.
modules/PRICE.sol:
Manual Review / VSCode
It is recommended to set exactly the same timeframe for accepting chainlink's prices for ohm and reserve asset.
#0 - Oighty
2022-09-06T18:36:20Z
Duplicate of #391
#1 - 0xean
2022-09-19T13:20:26Z
closing as dupe
🌟 Selected for report: reassor
Contract OlympusGovernance allows controlling protocol through on-chain governing. The issue is that once proposal is active it does not expire, which means that until the new proposal will be selected, anyone can vote on existing one and potentially execute it when it might cause harm to the protocol.
Scenario:
Governance.sol:
Manual Review / VSCode
It is recommended to add expiration for the active proposal for example 2 weeks. After that time it should be possible to reject proposal and users should be able to reclaim VOTES tokens.
#0 - fullyallocated
2022-09-04T03:11:09Z
Duplicate of #376
#1 - 0xean
2022-09-20T00:47:08Z
I don't think this is an exact dupe of #376
I believe the warden is simply stating that an active proposal stays active if not replaced. There is not expiration of a proposal once it becomes active, so theoretically if the governance process is inactive a very stale proposal could get executed.
11.0311 DAI - $11.03
https://github.com/code-423n4/2022-08-olympus/blob/277535739c465c75d37c33d706ab76365df2aade/src/modules/PRICE.sol#L161 https://github.com/code-423n4/2022-08-olympus/blob/277535739c465c75d37c33d706ab76365df2aade/src/modules/PRICE.sol#L170
Protocol uses Chainlink as an oracle for retrieving prices for the assets.
Function OlympusPrice.getCurrentPrice retrieves price using latestRoundData but the implementation is missing essential security checks which can result in stale and incorrect prices being returned.
modules/PRICE.sol:
Manual Review / VSCode
It is recommended to add checks on the returned data of latestRoundData with proper revert messages if the price is stale or the round is incomplete, for example:
( roundId, rawPrice, , updateTime, answeredInRound ) = _ohmEthPriceFeedk.latestRoundData(); require(rawPrice > 0, "price <= 0"); require(updateTime != 0, "incomplete round"); require(answeredInRound >= roundId, "stale price");
#0 - Oighty
2022-09-06T18:57:25Z
Duplicate. See comment on #441.
🌟 Selected for report: zzzitron
Also found by: 0x040, 0x1f8b, 0x52, 0x85102, 0xDjango, 0xNazgul, 0xNineDec, 0xSky, 0xSmartContract, 0xkatana, 8olidity, Aymen0909, Bahurum, BipinSah, Bnke0x0, CRYP70, CertoraInc, Ch_301, Chandr, Chom, CodingNameKiki, Deivitto, DimSon, Diraco, ElKu, EthLedger, Funen, GalloDaSballo, Guardian, IllIllI, JansenC, Jeiwan, Lambda, LeoS, Margaret, MasterCookie, PPrieditis, PaludoX0, Picodes, PwnPatrol, RaymondFam, ReyAdmirado, Rohan16, Rolezn, Ruhum, Sm4rty, StevenL, The_GUILD, TomJ, Tomo, Trust, Waze, __141345__, ajtra, ak1, apostle0x01, aviggiano, bin2chen, bobirichman, brgltd, c3phas, cRat1st0s, carlitox477, cccz, ch13fd357r0y3r, cloudjunky, cryptphi, csanuragjain, d3e4, datapunk, delfin454000, devtooligan, dipp, djxploit, durianSausage, eierina, enckrish, erictee, fatherOfBlocks, gogo, grGred, hansfriese, hyh, ignacio, indijanc, itsmeSTYJ, ladboy233, lukris02, martin, medikko, mics, natzuu, ne0n, nxrblsrpr, okkothejawa, oyc_109, p_crypt0, pfapostol, prasantgupta52, rajatbeladiya, rbserver, reassor, ret2basic, robee, rokinot, rvierdiiev, shenwilly, sikorico, sorrynotsorry, tnevler, tonisives, w0Lfrum, yixxas
60.6664 DAI - $60.67
Low
Multiple contracts do not check for zero addresses which might lead to loss of funds, failed transactions and can break the protocol functionality.
Kernel.sol:
kernel_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L65target_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L235addr_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L439addr_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L451modules/MINTR.sol:
ohm_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/MINTR.sol#L15modules/RANGE.sol:
tokens_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L79policies/Heart.sol:
kernel_, operator_ and rewardToken_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L55-L57token_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L140policies/Governance.sol:
kernel_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L59policies/BondCallback.sol:
kernel_, aggregator_ and _ohm - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L39-L41teller_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L83policies/Operator.sol:
kernel_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Operator.sol#L93Manual Review / VSCode
It is recommended to add zero address checks for listed parameters.
Low
Changing critical addresses such as ownership should be a two-step process where the first transaction (from the old/current address) registers the new address (i.e. grants ownership) and the second transaction (from the new address) replaces the old address with the new one. This gives an opportunity to recover from incorrect addresses mistakenly used in the first step. If not, contract functionality might become inaccessible.
Kernel.sol:
Manual Review / VSCode
It is recommended to implement two-step process for changing ownership.
Low
Multiple contracts are not implementing events for critical functions. Lack of events makes it difficult for off-chain applications to monitor the protocol.
policies/Heart.sol:
resetBeat function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L130toggleBeat function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L135withdrawUnspentRewards function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L150policies/Governance.sol:
reclaimVotes function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L295policies/BondCallback.sol:
whitelist function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L83callback function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L100batchToTreasury function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L152setOperator function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/BondCallback.sol#L190policies/Operator.sol:
setBondContracts function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Operator.sol#L586regenerate function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Operator.sol#L618toggleActive function event - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Operator.sol#L624Manual Review / VSCode
It is recommended to add missing events to listed functions.
Non-Critical
Protocol implements multiple events but does not properly index parameters for all of them. Lack of indexed parameters for events makes it difficult for off-chain applications to monitor the protocol.
modules/PRICE.sol:
movingAverageDuration_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L27observationFrequency_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L28modules/RANGE.sol:
high_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L20high_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L21high_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L22high_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L23policies/Heart.sol:
to_ - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Heart.sol#L29Manual Review / VSCode
It is recommended to add indexed keyword to listed events parameters.
Non-Critical
Contracts are missing pause functionality that could be used in case of security incidents and would block executing selected functions while the contract is paused.
Manual Review / VSCode
It is recommended to add functionality for pausing contracts and go through all publicly/externally accessible functions to decide which one should be forbidden from running while the contracts are paused.
Non-Critical
Using the latest versions might make contracts susceptible to undiscovered compiler bugs.
Kernel.sol:pragma solidity 0.8.15; utils/KernelUtils.sol:pragma solidity 0.8.15; modules/TRSRY.sol:pragma solidity 0.8.15; modules/VOTES.sol:pragma solidity 0.8.15; modules/MINTR.sol:pragma solidity 0.8.15; modules/PRICE.sol:pragma solidity 0.8.15; modules/INSTR.sol:pragma solidity 0.8.15; modules/RANGE.sol:pragma solidity 0.8.15; policies/PriceConfig.sol:pragma solidity 0.8.15; policies/Governance.sol:pragma solidity 0.8.15; policies/Heart.sol:pragma solidity 0.8.15; policies/Operator.sol:pragma solidity 0.8.15; policies/VoterRegistration.sol:pragma solidity 0.8.15; policies/TreasuryCustodian.sol:pragma solidity 0.8.15; policies/BondCallback.sol:pragma solidity 0.8.15;
Manual Review / VSCode
It is recommended to use more stable and tested solidity version such as 0.8.10.
Non-Critical
Multiple contracts are missing natspec comments which makes code more difficult to read and prone to errors.
Kernel.sol:
@param newKernel_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L76@param activate_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L126@param keycode_ and @return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L131@param action_ and @param target_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L235@param newKernel_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L351@param role_ and @param addr_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L439@param role_ and @param addr_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/Kernel.sol#L451utils/KernelUtils.sol:
modules/INSTR.sol:
@param instructionId_ and @return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/INSTR.sol#L37@param instructions_ and @return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/INSTR.sol#L42modules/MINTR.sol:
modules/PRICE.sol:
@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L154@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L183@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/PRICE.sol#L190modules/RANGE.sol:
@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L275@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L281@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L291@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L302@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L320@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L330@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/RANGE.sol#L340modules/TRSRY.sol:
@inheritdoc natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L47@inheritdoc natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L51@param withdrawer_, @param token_, and @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L64@param to_, @param token_, @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L75@param token_ and @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L92@param token_ and @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L105@param token_, @param debtor_, @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/TRSRY.sol#L122modules/VOTES.sol:
@param to_ and @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/VOTES.sol#L45@param from_, @param to_ and @param amount_ natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/modules/VOTES.sol#L51policies/TreasuryCustodian.sol:
policies/VoterRegistration.sol:
policies/PriceConfig.sol:
policies/Heart.sol:
policies/Governance.sol:
@param proposalId_ and @return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L145@return natspec - https://github.com/code-423n4/2022-08-olympus/blob/2a0b515012b4a40076f6eac487f7816aafb8724a/src/policies/Governance.sol#L151policies/BondCallback.sol:
policies/Operator.sol:
Manual Review / VSCode
It is recommended to add missing natspec comments.