diff --git a/data/Sm4rty-Q.md b/data/Sm4rty-Q.md index 74984b2..3b3e606 100644 --- a/data/Sm4rty-Q.md +++ b/data/Sm4rty-Q.md @@ -73,3 +73,118 @@ src/policies/Operator.sol:69: OlympusPrice internal PRICE; ### References: [https://code4rena.com/reports/2022-05-sturdy#n-08-variable-names-that-consist-of-all-capital-letters-should-be-reserved-for-constimmutable-variables](https://code4rena.com/reports/2022-05-sturdy#n-08-variable-names-that-consist-of-all-capital-letters-should-be-reserved-for-constimmutable-variables) + +----- + +## 5.Multiple initialization due to initialize function not having initializer modifier. + +### Description + +The attacker can initialize the contract, take malicious actions, and allow it to be re-initialized by the project without any error being noticed. +### Instances +[PriceConfig.sol:L45](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/PriceConfig.sol#L45) +[Operator.sol:L598](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L598) +[IOperator.sol:L125](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/interfaces/IOperator.sol#L125) +``` +// actual codes used +policies/PriceConfig.sol:45: function initialize(uint256[] memory startObservations_, uint48 lastObservationTime_) +policies/Operator.sol:598: function initialize() external onlyRole("operator_admin") { +policies/interfaces/IOperator.sol:125: function initialize() external; +``` +## 6. The non Reentrant modifier should occur before all other modifiers + +### Instances +[Operator.sol:L276](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L276) +``` +src/policies/Operator.sol:276: ) external override onlyWhileActive nonReentrant returns (uint256 amountOut) { +``` + +### Recommended Mitigation Steps +using the nonReentrant modifier before onlyWhileActive modifier + +----- +## 1. Use of Block.timestamp + +`Impact - Non-Critical` + +Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts. + +### Instances +[PRICE.sol:L143](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/PRICE.sol#L143) +[PRICE.sol:L146](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/PRICE.sol#L146) +[PRICE.sol:L165](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/PRICE.sol#L165) +[PRICE.sol:L171](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/PRICE.sol#L171) +[PRICE.sol:L215](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/PRICE.sol#L215) +[RANGE.sol:L85](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L85) +[RANGE.sol:L92](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L92) +[RANGE.sol:L136](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L136) +[RANGE.sol:L138](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L138) +[RANGE.sol:L148](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L148) +[RANGE.sol:L150](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L150) +[RANGE.sol:L191](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L191) +[RANGE.sol:L200](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L200) +[RANGE.sol:L207](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L207) +[RANGE.sol:L231](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L231) +[RANGE.sol:L233](https://github.com/code-423n4/2022-08-olympus/tree/main/src/modules/RANGE.sol#L233) +[Heart.sol:L63](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Heart.sol#L63) +[Heart.sol:L94](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Heart.sol#L94) +[Heart.sol:L108](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Heart.sol#L108) +[Heart.sol:L131](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Heart.sol#L131) +[Operator.sol:L128](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L128) +[Operator.sol:L210](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L210) +[Operator.sol:L216](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L216) +[Operator.sol:L404](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L404) +[Operator.sol:L456](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L456) +[Operator.sol:L708](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L708) +[Operator.sol:L720](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Operator.sol#L720) +[Governance.sol:L171](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L171) +[Governance.sol:L212](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L212) +[Governance.sol:L227](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L227) +[Governance.sol:L231](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L231) +[Governance.sol:L235](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L235) +[Governance.sol:L272](https://github.com/code-423n4/2022-08-olympus/tree/main/src/policies/Governance.sol#L272) +``` +src/modules/PRICE.sol:143: lastObservationTime = uint48(block.timestamp); +src/modules/PRICE.sol:146: emit NewObservation(block.timestamp, currentPrice, _movingAverage); +src/modules/PRICE.sol:165: if (updatedAt < block.timestamp - 3 * uint256(observationFrequency)) +src/modules/PRICE.sol:171: if (updatedAt < block.timestamp - uint256(observationFrequency)) +src/modules/PRICE.sol:215: if (startObservations_.length != numObs || lastObservationTime_ > uint48(block.timestamp)) +src/modules/RANGE.sol:85: lastActive: uint48(block.timestamp), +src/modules/RANGE.sol:92: lastActive: uint48(block.timestamp), +src/modules/RANGE.sol:136: _range.high.lastActive = uint48(block.timestamp); +src/modules/RANGE.sol:138: emit WallDown(true, block.timestamp, capacity_); +src/modules/RANGE.sol:148: _range.low.lastActive = uint48(block.timestamp); +src/modules/RANGE.sol:150: emit WallDown(false, block.timestamp, capacity_); +src/modules/RANGE.sol:191: lastActive: uint48(block.timestamp), +src/modules/RANGE.sol:200: lastActive: uint48(block.timestamp), +src/modules/RANGE.sol:207: emit WallUp(high_, block.timestamp, capacity_); +src/modules/RANGE.sol:231: emit CushionDown(high_, block.timestamp); +src/modules/RANGE.sol:233: emit CushionUp(high_, block.timestamp, marketCapacity_); +src/policies/Heart.sol:63: lastBeat = block.timestamp; +src/policies/Heart.sol:94: if (block.timestamp < lastBeat + frequency()) revert Heart_OutOfCycle(); +src/policies/Heart.sol:108: emit Beat(block.timestamp); +src/policies/Heart.sol:131: lastBeat = block.timestamp - frequency(); +src/policies/Operator.sol:128: lastRegen: uint48(block.timestamp), +src/policies/Operator.sol:210: uint48(block.timestamp) >= RANGE.lastActive(true) + uint48(config_.regenWait) && +src/policies/Operator.sol:216: uint48(block.timestamp) >= RANGE.lastActive(false) + uint48(config_.regenWait) && +src/policies/Operator.sol:404: conclusion: uint48(block.timestamp + config_.cushionDuration), +src/policies/Operator.sol:456: conclusion: uint48(block.timestamp + config_.cushionDuration), +src/policies/Operator.sol:708: _status.high.lastRegen = uint48(block.timestamp); +src/policies/Operator.sol:720: _status.low.lastRegen = uint48(block.timestamp); +src/policies/Governance.sol:171: block.timestamp, +src/policies/Governance.sol:212: if (block.timestamp > proposal.submissionTimestamp + ACTIVATION_DEADLINE) { +src/policies/Governance.sol:227: if (block.timestamp < activeProposal.activationTimestamp + GRACE_PERIOD) { +src/policies/Governance.sol:231: activeProposal = ActivatedProposal(proposalId_, block.timestamp); +src/policies/Governance.sol:235: emit ProposalActivated(proposalId_, block.timestamp); +src/policies/Governance.sol:272: if (block.timestamp < activeProposal.activationTimestamp + EXECUTION_TIMELOCK) { + +``` + +### Recommended Mitigation Steps +Block timestamps should not be used for entropy or generating random numbers—i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state. + + +Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number. + +----- +