diff --git a/reports/adhoc-sol-files-highs-only-report.json b/reports/adhoc-sol-files-highs-only-report.json
index 8a953244..8dd778bf 100644
--- a/reports/adhoc-sol-files-highs-only-report.json
+++ b/reports/adhoc-sol-files-highs-only-report.json
@@ -216,6 +216,7 @@
"out-of-order-retryable",
"constant-function-changing-state",
"function-selector-collision",
- "unchecked-low-level-call"
+ "unchecked-low-level-call",
+ "state-change-after-external-call"
]
}
\ No newline at end of file
diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md
index c6a976fd..1266a31d 100644
--- a/reports/ccip-functions-report.md
+++ b/reports/ccip-functions-report.md
@@ -11,6 +11,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
- [H-1: Unprotected initializer](#h-1-unprotected-initializer)
- [H-2: Contract Name Reused in Different Files](#h-2-contract-name-reused-in-different-files)
- [H-3: Uninitialized State Variables](#h-3-uninitialized-state-variables)
+ - [H-4: External call is followed by a state variable change](#h-4-external-call-is-followed-by-a-state-variable-change)
- [Low Issues](#low-issues)
- [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners)
- [L-2: `ecrecover` is susceptible to signature malleability](#l-2-ecrecover-is-susceptible-to-signature-malleability)
@@ -105,7 +106,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| Category | No. of Issues |
| --- | --- |
-| High | 3 |
+| High | 4 |
| Low | 18 |
@@ -492,6 +493,53 @@ Solidity does initialize variables by default when you declare them, however it'
+## H-4: External call is followed by a state variable change
+
+In most cases it is a best practice to perform the state change before making an external call to avoid a potential re-entrancy attack.
+
+6 Found Instances
+
+
+- Found in src/v0.8/functions/dev/v1_X/FunctionsBilling.sol [Line: 403](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsBilling.sol#L403)
+
+ ```solidity
+ IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
+ ```
+
+- Found in src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol [Line: 519](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol#L519)
+
+ ```solidity
+ IFunctionsBilling(request.coordinator).deleteCommitment(requestId);
+ ```
+
+- Found in src/v0.8/functions/v1_0_0/FunctionsBilling.sol [Line: 342](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsBilling.sol#L342)
+
+ ```solidity
+ IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
+ ```
+
+- Found in src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol [Line: 519](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol#L519)
+
+ ```solidity
+ IFunctionsBilling(request.coordinator).deleteCommitment(requestId);
+ ```
+
+- Found in src/v0.8/functions/v1_1_0/FunctionsBilling.sol [Line: 360](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_1_0/FunctionsBilling.sol#L360)
+
+ ```solidity
+ IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
+ ```
+
+- Found in src/v0.8/functions/v1_3_0/FunctionsBilling.sol [Line: 401](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_3_0/FunctionsBilling.sol#L401)
+
+ ```solidity
+ IFunctionsSubscriptions(address(_getRouter())).oracleWithdraw(transmitters[i], balance);
+ ```
+
+6 Found Instances
+
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 24](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L24)
+
+ ```solidity
+ s_actor.hello();
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 33](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L33)
+
+ ```solidity
+ s_actor.hello();
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 52](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L52)
+
+ ```solidity
+ s_actor.hello();
+ ```
+
+- Found in src/Trump.sol [Line: 342](../tests/contract-playground/src/Trump.sol#L342)
+
+ ```solidity
+ uniswapV2Pair = IUniswapV2Factory(uniswapV2Router.factory()).createPair(address(this), uniswapV2Router.WETH());
+ ```
+
+- Found in src/Trump.sol [Line: 343](../tests/contract-playground/src/Trump.sol#L343)
+
+ ```solidity
+ uniswapV2Router.addLiquidityETH{value: address(this).balance}(address(this),balanceOf(address(this)),0,0,owner(),block.timestamp);
+ ```
+
+- Found in src/Trump.sol [Line: 344](../tests/contract-playground/src/Trump.sol#L344)
+
+ ```solidity
+ IERC20(uniswapV2Pair).approve(address(uniswapV2Router), type(uint).max);
+ ```
+
+24 Found Instances
+25 Found Instances
- Found in src/CallGraphTests.sol [Line: 7](../tests/contract-playground/src/CallGraphTests.sol#L7)
@@ -4255,6 +4304,12 @@ Use descriptive reason strings or custom errors for revert paths.
revert();
```
+- Found in src/StateChangeAfterExternalCall.sol [Line: 17](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L17)
+
+ ```solidity
+ require(actor != address(0));
+ ```
+
- Found in src/StateShadowing.sol [Line: 8](../tests/contract-playground/src/StateShadowing.sol#L8)
```solidity
@@ -6184,7 +6239,7 @@ Argument to `assert()` modifies the state. Use `require` for invariants modifyin
Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local variable to hold the loop computation result.
-14 Found Instances
+15 Found Instances
- Found in src/CacheArrayLength.sol [Line: 25](../tests/contract-playground/src/CacheArrayLength.sol#L25)
@@ -6253,6 +6308,12 @@ Invoking `SSTORE`operations in loops may lead to Out-of-gas errors. Use a local
for (uint256 id = 0; id < 10; ++id) {
```
+- Found in src/StateChangeAfterExternalCall.sol [Line: 47](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L47)
+
+ ```solidity
+ for (uint256 i = 0; i < s_useMe; ++i) {
+ ```
+
- Found in src/eth2/DepositContract.sol [Line: 70](../tests/contract-playground/src/eth2/DepositContract.sol#L70)
```solidity
@@ -6690,7 +6751,7 @@ State variables that are not updated following deployment should be declared con
State variable changes in this function but no event is emitted.
-108 Found Instances
+113 Found Instances
- Found in src/AbstractContract.sol [Line: 6](../tests/contract-playground/src/AbstractContract.sol#L6)
@@ -7029,6 +7090,36 @@ State variable changes in this function but no event is emitted.
function deductMassFundsV3() external returns(uint256[] memory failedIds) {
```
+- Found in src/StateChangeAfterExternalCall.sol [Line: 22](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L22)
+
+ ```solidity
+ function badSituation1() external {
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 31](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L31)
+
+ ```solidity
+ function badSituation2() external {
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 42](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L42)
+
+ ```solidity
+ function badSituation3() external {
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 57](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L57)
+
+ ```solidity
+ function goodSituation1() external {
+ ```
+
+- Found in src/StateChangeAfterExternalCall.sol [Line: 66](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L66)
+
+ ```solidity
+ function goodSituation2() external {
+ ```
+
- Found in src/StateVariableCouldBeDeclaredConstant.sol [Line: 39](../tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol#L39)
```solidity
@@ -7349,7 +7440,7 @@ State variable changes in this function but no event is emitted.
State variables that are should be declared immutable to save gas. Add the `immutable` attribute to state variables that are only changed in the constructor
-32 Found Instances
+33 Found Instances
- Found in src/ArbitraryTransferFrom.sol [Line: 9](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L9)
@@ -7424,6 +7515,12 @@ State variables that are should be declared immutable to save gas. Add the `immu
uint256 public gas;
```
+- Found in src/StateChangeAfterExternalCall.sol [Line: 14](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L14)
+
+ ```solidity
+ MaliciousActor s_actor;
+ ```
+
- Found in src/StateVariableCouldBeDeclaredConstant.sol [Line: 11](../tests/contract-playground/src/StateVariableCouldBeDeclaredConstant.sol#L11)
```solidity
diff --git a/reports/report.sarif b/reports/report.sarif
index 23225827..e76fba55 100644
--- a/reports/report.sarif
+++ b/reports/report.sarif
@@ -3754,6 +3754,81 @@
},
"ruleId": "unchecked-low-level-call"
},
+ {
+ "level": "warning",
+ "locations": [
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 15,
+ "byteOffset": 588
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 15,
+ "byteOffset": 735
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 15,
+ "byteOffset": 1272
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/Trump.sol"
+ },
+ "region": {
+ "byteLength": 110,
+ "byteOffset": 11731
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/Trump.sol"
+ },
+ "region": {
+ "byteLength": 129,
+ "byteOffset": 11851
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/Trump.sol"
+ },
+ "region": {
+ "byteLength": 71,
+ "byteOffset": 11990
+ }
+ }
+ }
+ ],
+ "message": {
+ "text": "In most cases it is a best practice to perform the state change before making an external call to avoid a potential re-entrancy attack."
+ },
+ "ruleId": "state-change-after-external-call"
+ },
{
"level": "note",
"locations": [
@@ -6806,6 +6881,17 @@
}
}
},
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 7,
+ "byteOffset": 428
+ }
+ }
+ },
{
"physicalLocation": {
"artifactLocation": {
@@ -10188,6 +10274,17 @@
}
}
},
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 154,
+ "byteOffset": 1144
+ }
+ }
+ },
{
"physicalLocation": {
"artifactLocation": {
@@ -11536,6 +11633,61 @@
}
}
},
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 13,
+ "byteOffset": 530
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 13,
+ "byteOffset": 677
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 13,
+ "byteOffset": 885
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 14,
+ "byteOffset": 1331
+ }
+ }
+ },
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 14,
+ "byteOffset": 1480
+ }
+ }
+ },
{
"physicalLocation": {
"artifactLocation": {
@@ -12249,6 +12401,17 @@
}
}
},
+ {
+ "physicalLocation": {
+ "artifactLocation": {
+ "uri": "src/StateChangeAfterExternalCall.sol"
+ },
+ "region": {
+ "byteLength": 7,
+ "byteOffset": 377
+ }
+ }
+ },
{
"physicalLocation": {
"artifactLocation": {
diff --git a/reports/templegold-report.md b/reports/templegold-report.md
index 0a6db60f..444a8510 100644
--- a/reports/templegold-report.md
+++ b/reports/templegold-report.md
@@ -17,6 +17,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
- [H-7: Weak Randomness](#h-7-weak-randomness)
- [H-8: Deletion from a nested mappping.](#h-8-deletion-from-a-nested-mappping)
- [H-9: Contract locks Ether without a withdraw function.](#h-9-contract-locks-ether-without-a-withdraw-function)
+ - [H-10: External call is followed by a state variable change](#h-10-external-call-is-followed-by-a-state-variable-change)
- [Low Issues](#low-issues)
- [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners)
- [L-2: `ecrecover` is susceptible to signature malleability](#l-2-ecrecover-is-susceptible-to-signature-malleability)
@@ -197,7 +198,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati
| Category | No. of Issues |
| --- | --- |
-| High | 9 |
+| High | 10 |
| Low | 27 |
@@ -548,6 +549,155 @@ It appears that the contract includes a payable function to accept Ether but lac
+## H-10: External call is followed by a state variable change
+
+In most cases it is a best practice to perform the state change before making an external call to avoid a potential re-entrancy attack.
+
+23 Found Instances
+
+
+- Found in contracts/amo/Ramos.sol [Line: 195](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L195)
+
+ ```solidity
+ protocolToken.approve(previousVault, 0);
+ ```
+
+- Found in contracts/amo/Ramos.sol [Line: 196](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L196)
+
+ ```solidity
+ quoteToken.approve(previousVault, 0);
+ ```
+
+- Found in contracts/core/MultiOtcOffer.sol [Line: 68](../tests/2024-07-templegold/protocol/contracts/core/MultiOtcOffer.sol#L68)
+
+ ```solidity
+ uint256 scaleDecimals = marketInfo.offerPricingToken == OfferPricingToken.UserBuyToken
+ ```
+
+- Found in contracts/core/OpsManager.sol [Line: 36](../tests/2024-07-templegold/protocol/contracts/core/OpsManager.sol#L36)
+
+ ```solidity
+ templeExposure.setMinterState(address(this), true);
+ ```
+
+- Found in contracts/core/OtcOffer.sol [Line: 91](../tests/2024-07-templegold/protocol/contracts/core/OtcOffer.sol#L91)
+
+ ```solidity
+ uint256 scaleDecimals = offerPricingToken == OfferPricingToken.UserBuyToken
+ ```
+
+- Found in contracts/fakes/templegold/TempleGoldStakingMock.sol [Line: 158](../tests/2024-07-templegold/protocol/contracts/fakes/templegold/TempleGoldStakingMock.sol#L158)
+
+ ```solidity
+ uint256 amount = previousStaking.migrateWithdraw(msg.sender, index);
+ ```
+
+- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 52](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L52)
+
+ ```solidity
+ _checkpointDaiBalance(daiToken.balanceOf(address(this)));
+ ```
+
+- Found in contracts/governance/ElderElection.sol [Line: 74](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L74)
+
+ ```solidity
+ templars.checkExists(discordId);
+ ```
+
+- Found in contracts/governance/ElderElection.sol [Line: 87](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L87)
+
+ ```solidity
+ templars.checkExists(discordId);
+ ```
+
+- Found in contracts/governance/TemplarMetadata.sol [Line: 32](../tests/2024-07-templegold/protocol/contracts/governance/TemplarMetadata.sol#L32)
+
+ ```solidity
+ templars.checkExists(discordId);
+ ```
+
+- Found in contracts/templegold/SpiceAuction.sol [Line: 159](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L159)
+
+ ```solidity
+ uint256 balance = IERC20(auctionToken).balanceOf(address(this));
+ ```
+
+- Found in contracts/templegold/SpiceAuction.sol [Line: 194](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L194)
+
+ ```solidity
+ uint256 _bidTokenAmountBefore = IERC20(bidToken).balanceOf(_recipient);
+ ```
+
+- Found in contracts/templegold/SpiceAuction.sol [Line: 196](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L196)
+
+ ```solidity
+ uint256 _bidTokenAmountAfter = IERC20(bidToken).balanceOf(_recipient);
+ ```
+
+- Found in contracts/v2/TreasuryReservesVault.sol [Line: 148](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L148)
+
+ ```solidity
+ if (_tpiOracle.treasuryPriceIndex() == 0) revert CommonEventsAndErrors.InvalidParam();
+ ```
+
+- Found in contracts/v2/TreasuryReservesVault.sol [Line: 296](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L296)
+
+ ```solidity
+ _outstandingDebt = borrowTokens[_token].dToken.burnAll(strategy);
+ ```
+
+- Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 55](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L55)
+
+ ```solidity
+ IMakerDaoVatLike vat = IMakerDaoVatLike(daiJoin.vat());
+ ```
+
+- Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 57](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L57)
+
+ ```solidity
+ vat.hope(address(daiJoin));
+ ```
+
+- Found in contracts/v2/strategies/DsrBaseStrategy.sol [Line: 58](../tests/2024-07-templegold/protocol/contracts/v2/strategies/DsrBaseStrategy.sol#L58)
+
+ ```solidity
+ vat.hope(address(pot));
+ ```
+
+- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 173](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L173)
+
+ ```solidity
+ circuitBreakerProxy.preCheck(
+ ```
+
+- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 213](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L213)
+
+ ```solidity
+ circuitBreakerProxy.preCheck(
+ ```
+
+- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 351](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L351)
+
+ ```solidity
+ treasuryReservesVault.repay(templeToken, totalCollateralClaimed, address(tlcStrategy));
+ ```
+
+- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 394](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L394)
+
+ ```solidity
+ daiToken.approve(previousTrv, 0);
+ ```
+
+- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 397](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L397)
+
+ ```solidity
+ address _trv = address(tlcStrategy.treasuryReservesVault());
+ ```
+
+