Report for issue #233 updated by Sm4rty

code-423n4 · Sep 1, 2022 · cc9ebd5 · cc9ebd5
1 parent 10cca18
commit cc9ebd5
Showing 1 changed file with 115 additions and 0 deletions.
diff --git 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.
+
+-----
+