diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index 1c3ced3d..d398e416 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -106,6 +106,7 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), ] } @@ -117,6 +118,7 @@ pub fn get_all_detectors_names() -> Vec { #[derive(Debug, PartialEq, EnumString, Display)] #[strum(serialize_all = "kebab-case")] pub(crate) enum IssueDetectorNamePool { + EmitAfterExternalCall, StateChangeAfterExternalCall, StateVariableCouldBeDeclaredImmutable, MultiplePlaceholders, @@ -215,6 +217,9 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { + Some(Box::::default()) + } IssueDetectorNamePool::StateChangeAfterExternalCall => { Some(Box::::default()) } diff --git a/aderyn_core/src/detect/low/emit_after_ext_call.rs b/aderyn_core/src/detect/low/emit_after_ext_call.rs new file mode 100644 index 00000000..25634a86 --- /dev/null +++ b/aderyn_core/src/detect/low/emit_after_ext_call.rs @@ -0,0 +1,220 @@ +use std::{ + collections::{BTreeMap, BTreeSet, HashSet}, + error::Error, +}; + +use crate::{ + ast::NodeID, + capture, + context::{ + browser::{ExtractFunctionCalls, Peek}, + flow::{Cfg, CfgNodeDescriptor, CfgNodeId}, + workspace_context::WorkspaceContext, + }, + detect::{ + detector::{IssueDetector, IssueDetectorNamePool, IssueSeverity}, + helpers, + }, +}; +use eyre::{eyre, Result}; + +#[derive(Default)] +pub struct EmitAfterExternalCallDetector { + // Keys are: [0] source file name, [1] line number, [2] character location of node. + // Do not add items manually, use `capture!` to add nodes to this BTreeMap. + found_instances: BTreeMap<(String, usize, String), NodeID>, + hints: BTreeMap<(String, usize, String), String>, +} + +impl IssueDetector for EmitAfterExternalCallDetector { + fn detect(&mut self, context: &WorkspaceContext) -> Result> { + // When you have found an instance of the issue, + // use the following macro to add it to `found_instances`: + // + // capture!(self, context, item); + // capture!(self, context, item, "hint"); + + for func in helpers::get_implemented_external_and_public_functions(context) { + let (cfg, start, _) = + Cfg::from_function_body(context, func).ok_or(eyre!("corrupted function"))?; + + // Discover external calls + let external_call_sites = find_external_call_sites(context, &cfg, start); + + // For each external call, figure out if it's followed by a state change + for external_call_site in external_call_sites { + // Capture the external call + let external_call_cfg_node = + cfg.nodes.get(&external_call_site).expect("cfg is corrupted!"); + + let Some(external_call_ast_node) = external_call_cfg_node.reflect(context) else { + continue; + }; + + // Discover state changes that follow the external call + let emit_sites = find_emit_sites(&cfg, external_call_site); + let mut hint = "Emission happens at: ".to_string(); + let mut emissions_found = false; + + for emit_site in emit_sites { + // There is no way to tell is the state change took place after the event if + // both are found at the same place + if emit_site != external_call_site { + emissions_found = true; + let emit_cfg_node = cfg.nodes.get(&emit_site).expect("cfg is corrupted"); + + if let Some(emit_ast_node) = emit_cfg_node.reflect(context) { + if let Some(state_change_code) = emit_ast_node.peek(context) { + hint.push('`'); + hint.push_str(&state_change_code); + hint.push('`'); + hint.push_str(", "); + } + } + } + } + + if emissions_found { + hint.pop(); + hint.pop(); + + capture!(self, context, external_call_ast_node, hint); + } + } + } + + Ok(!self.found_instances.is_empty()) + } + + fn severity(&self) -> IssueSeverity { + IssueSeverity::Low + } + + fn title(&self) -> String { + String::from("External call is followed by an emit statement") + } + + fn description(&self) -> String { + String::from("In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack.") + } + + fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { + self.found_instances.clone() + } + + fn hints(&self) -> BTreeMap<(String, usize, String), String> { + self.hints.clone() + } + + fn name(&self) -> String { + IssueDetectorNamePool::EmitAfterExternalCall.to_string() + } +} + +fn find_emit_sites(cfg: &Cfg, start_node: CfgNodeId) -> BTreeSet { + let mut visited = Default::default(); + let mut emit_statements_sites = Default::default(); + + fn _find_emit_sites( + cfg: &Cfg, + visited: &mut HashSet, + curr_node: CfgNodeId, + emit_sites: &mut HashSet, + ) -> Option<()> { + if visited.contains(&curr_node) { + return Some(()); + } + + visited.insert(curr_node); + + // Check if `curr_node` is an external call site + let curr_cfg_node = cfg.nodes.get(&curr_node)?; + + if let CfgNodeDescriptor::EmitStatement(_) = &curr_cfg_node.nd { + emit_sites.insert(curr_node); + } + + // Continue the recursion + for child in curr_node.children(cfg) { + _find_emit_sites(cfg, visited, child, emit_sites); + } + + Some(()) + } + + _find_emit_sites(cfg, &mut visited, start_node, &mut emit_statements_sites); + + emit_statements_sites.into_iter().collect() +} + +fn find_external_call_sites( + context: &WorkspaceContext, + cfg: &Cfg, + start_node: CfgNodeId, +) -> BTreeSet { + let mut visited = Default::default(); + let mut external_call_sites = Default::default(); + + fn _find_external_call_sites( + context: &WorkspaceContext, + cfg: &Cfg, + visited: &mut HashSet, + curr_node: CfgNodeId, + external_call_sites: &mut HashSet, + ) -> Option<()> { + if visited.contains(&curr_node) { + return Some(()); + } + + visited.insert(curr_node); + + // Check if `curr_node` is an external call site + let curr_cfg_node = cfg.nodes.get(&curr_node)?; + + // Grab the AST version of the Cfg Node + if let Some(curr_ast_node) = curr_cfg_node.reflect(context) { + let function_calls = ExtractFunctionCalls::from(curr_ast_node).extracted; + + if function_calls.iter().any(|f| f.is_external_call()) { + external_call_sites.insert(curr_node); + } + } + + // Continue the recursion + for child in curr_node.children(cfg) { + _find_external_call_sites(context, cfg, visited, child, external_call_sites); + } + + Some(()) + } + + _find_external_call_sites(context, cfg, &mut visited, start_node, &mut external_call_sites); + + external_call_sites.into_iter().collect() +} + +#[cfg(test)] +mod emit_after_external_call_tests { + use serial_test::serial; + + use crate::detect::{ + detector::IssueDetector, low::emit_after_ext_call::EmitAfterExternalCallDetector, + }; + + #[test] + #[serial] + fn test_emit_after_external_call() { + let context = crate::detect::test_utils::load_solidity_source_unit( + "../tests/contract-playground/src/EmitAfterExternalCall.sol", + ); + + let mut detector = EmitAfterExternalCallDetector::default(); + let found = detector.detect(&context).unwrap(); + // assert that the detector found an issue + assert!(found); + // assert that the detector found the correct number of instances + assert_eq!(detector.instances().len(), 3); + // assert the severity is high + assert_eq!(detector.severity(), crate::detect::detector::IssueSeverity::Low); + } +} diff --git a/aderyn_core/src/detect/low/mod.rs b/aderyn_core/src/detect/low/mod.rs index 4091db08..2d97877c 100644 --- a/aderyn_core/src/detect/low/mod.rs +++ b/aderyn_core/src/detect/low/mod.rs @@ -10,6 +10,7 @@ pub(crate) mod dead_code; pub(crate) mod deprecated_oz_functions; pub(crate) mod division_before_multiplication; pub(crate) mod ecrecover; +pub(crate) mod emit_after_ext_call; pub(crate) mod empty_blocks; pub(crate) mod function_init_state_vars; pub(crate) mod function_pointer_in_constructor; @@ -56,6 +57,7 @@ pub use dead_code::DeadCodeDetector; pub use deprecated_oz_functions::DeprecatedOZFunctionsDetector; pub use division_before_multiplication::DivisionBeforeMultiplicationDetector; pub use ecrecover::EcrecoverDetector; +pub use emit_after_ext_call::EmitAfterExternalCallDetector; pub use empty_blocks::EmptyBlockDetector; pub use function_init_state_vars::FunctionInitializingStateDetector; pub use function_pointer_in_constructor::FucntionPointerInConstructorDetector; diff --git a/reports/ccip-functions-report.md b/reports/ccip-functions-report.md index 746aa162..8f7b3e38 100644 --- a/reports/ccip-functions-report.md +++ b/reports/ccip-functions-report.md @@ -31,6 +31,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-16: Costly operations inside loops.](#l-16-costly-operations-inside-loops) - [L-17: Unused Imports](#l-17-unused-imports) - [L-18: State variable changes but no event is emitted.](#l-18-state-variable-changes-but-no-event-is-emitted) + - [L-19: External call is followed by an emit statement](#l-19-external-call-is-followed-by-an-emit-statement) # Summary @@ -107,7 +108,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 4 | -| Low | 18 | +| Low | 19 | # High Issues @@ -2900,3 +2901,42 @@ State variable changes in this function but no event is emitted. +## L-19: External call is followed by an emit statement + +In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack. + +
4 Found Instances + + +- Found in src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol [Line: 158](../tests/ccip-contracts/contracts/src/v0.8/functions/dev/v1_X/FunctionsSubscriptions.sol#L158) + + Emission happens at: `emit FundsRecovered(to, amount)` + ```solidity + uint256 externalBalance = i_linkToken.balanceOf(address(this)); + ``` + +- 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) + + Emission happens at: `emit RequestTimedOut(requestId)` + ```solidity + IFunctionsBilling(request.coordinator).deleteCommitment(requestId); + ``` + +- Found in src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol [Line: 158](../tests/ccip-contracts/contracts/src/v0.8/functions/v1_0_0/FunctionsSubscriptions.sol#L158) + + Emission happens at: `emit FundsRecovered(to, amount)` + ```solidity + uint256 externalBalance = i_linkToken.balanceOf(address(this)); + ``` + +- 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) + + Emission happens at: `emit RequestTimedOut(requestId)` + ```solidity + IFunctionsBilling(request.coordinator).deleteCommitment(requestId); + ``` + +
+ + + diff --git a/reports/report.json b/reports/report.json index 380aaef8..13f7e91e 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 115, - "total_sloc": 4244 + "total_source_units": 116, + "total_sloc": 4286 }, "files_details": { "files_details": [ @@ -121,6 +121,10 @@ "file_path": "src/DynamicArrayLengthAssignment.sol", "n_sloc": 16 }, + { + "file_path": "src/EmitAfterExternalCall.sol", + "n_sloc": 42 + }, { "file_path": "src/EmptyBlocks.sol", "n_sloc": 48 @@ -469,7 +473,7 @@ }, "issue_count": { "high": 43, - "low": 45 + "low": 46 }, "high_issues": { "issues": [ @@ -1347,6 +1351,18 @@ "description": "When compiling contracts with certain development frameworks (for example: Truffle), having contracts with the same name across different files can lead to one being overwritten.", "detector_name": "reused-contract-name", "instances": [ + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 4, + "src": "66:14", + "src_char": "66:14" + }, + { + "contract_path": "src/StateChangeAfterExternalCall.sol", + "line_no": 4, + "src": "66:14", + "src_char": "66:14" + }, { "contract_path": "src/nested/1/Nested.sol", "line_no": 7, @@ -2101,6 +2117,12 @@ "description": "The condition has been determined to be either always true or always false due to the integer range in which we're operating.", "detector_name": "tautology-or-contradiction", "instances": [ + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 48, + "src": "1168:5", + "src_char": "1168:5" + }, { "contract_path": "src/TautologyOrContradiction.sol", "line_no": 13, @@ -4299,6 +4321,12 @@ "src": "1389:6", "src_char": "1389:6" }, + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 18, + "src": "424:7", + "src_char": "424:7" + }, { "contract_path": "src/ExternalCalls.sol", "line_no": 72, @@ -7359,6 +7387,12 @@ "src": "217:7", "src_char": "217:7" }, + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 15, + "src": "373:7", + "src_char": "373:7" + }, { "contract_path": "src/ExternalCalls.sol", "line_no": 25, @@ -7565,6 +7599,41 @@ "src_char": "186:10" } ] + }, + { + "title": "External call is followed by an emit statement", + "description": "In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack.", + "detector_name": "emit-after-external-call", + "instances": [ + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 25, + "src": "584:15", + "src_char": "584:15", + "hint": "Emission happens at: `emit SomeEvent()`" + }, + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 34, + "src": "735:15", + "src_char": "735:15", + "hint": "Emission happens at: `emit SomeEvent()`" + }, + { + "contract_path": "src/EmitAfterExternalCall.sol", + "line_no": 53, + "src": "1274:15", + "src_char": "1274:15", + "hint": "Emission happens at: `emit SomeEvent()`" + }, + { + "contract_path": "src/Trump.sol", + "line_no": 315, + "src": "10741:54", + "src_char": "10741:54", + "hint": "Emission happens at: `emit ClearToken(tokenAddress, tokens)`" + } + ] } ] }, @@ -7656,6 +7725,7 @@ "state-variable-changes-without-events", "state-variable-could-be-declared-immutable", "multiple-placeholders", - "state-change-after-external-call" + "state-change-after-external-call", + "emit-after-external-call" ] } \ No newline at end of file diff --git a/reports/report.md b/reports/report.md index 6aa95ade..93fefa66 100644 --- a/reports/report.md +++ b/reports/report.md @@ -97,6 +97,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-43: State variable changes but no event is emitted.](#l-43-state-variable-changes-but-no-event-is-emitted) - [L-44: State variable could be declared immutable](#l-44-state-variable-could-be-declared-immutable) - [L-45: Modifier has multiple placeholders.](#l-45-modifier-has-multiple-placeholders) + - [L-46: External call is followed by an emit statement](#l-46-external-call-is-followed-by-an-emit-statement) # Summary @@ -105,8 +106,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 115 | -| Total nSLOC | 4244 | +| .sol Files | 116 | +| Total nSLOC | 4286 | ## Files Details @@ -142,6 +143,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/DeprecatedOZFunctions.sol | 32 | | src/DivisionBeforeMultiplication.sol | 22 | | src/DynamicArrayLengthAssignment.sol | 16 | +| src/EmitAfterExternalCall.sol | 42 | | src/EmptyBlocks.sol | 48 | | src/EnumerableSetIteration.sol | 55 | | src/ExperimentalEncoder.sol | 4 | @@ -228,7 +230,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/reused_contract_name/ContractB.sol | 7 | | src/uniswap/UniswapV2Swapper.sol | 50 | | src/uniswap/UniswapV3Swapper.sol | 150 | -| **Total** | **4244** | +| **Total** | **4286** | ## Issue Summary @@ -236,7 +238,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 43 | -| Low | 45 | +| Low | 46 | # High Issues @@ -1160,8 +1162,20 @@ In some versions of Solidity, contracts compile with multiple constructors. The When compiling contracts with certain development frameworks (for example: Truffle), having contracts with the same name across different files can lead to one being overwritten. -
4 Found Instances +
6 Found Instances + + +- Found in src/EmitAfterExternalCall.sol [Line: 4](../tests/contract-playground/src/EmitAfterExternalCall.sol#L4) + + ```solidity + contract MaliciousActor { + ``` +- Found in src/StateChangeAfterExternalCall.sol [Line: 4](../tests/contract-playground/src/StateChangeAfterExternalCall.sol#L4) + + ```solidity + contract MaliciousActor { + ``` - Found in src/nested/1/Nested.sol [Line: 7](../tests/contract-playground/src/nested/1/Nested.sol#L7) @@ -1978,8 +1992,14 @@ Potentially mistakened `=+` for `+=` or `=-` for `-=`. Please include a space in The condition has been determined to be either always true or always false due to the integer range in which we're operating. -
2 Found Instances +
3 Found Instances + + +- Found in src/EmitAfterExternalCall.sol [Line: 48](../tests/contract-playground/src/EmitAfterExternalCall.sol#L48) + ```solidity + for (uint256 i = 0; i < 0; ++i) { + ``` - Found in src/TautologyOrContradiction.sol [Line: 13](../tests/contract-playground/src/TautologyOrContradiction.sol#L13) @@ -4217,7 +4237,7 @@ Index event fields make the field more quickly accessible to off-chain tools tha Use descriptive reason strings or custom errors for revert paths. -
25 Found Instances +
26 Found Instances - Found in src/CallGraphTests.sol [Line: 7](../tests/contract-playground/src/CallGraphTests.sol#L7) @@ -4262,6 +4282,12 @@ Use descriptive reason strings or custom errors for revert paths. revert(); ``` +- Found in src/EmitAfterExternalCall.sol [Line: 18](../tests/contract-playground/src/EmitAfterExternalCall.sol#L18) + + ```solidity + require(actor != address(0)); + ``` + - Found in src/ExternalCalls.sol [Line: 72](../tests/contract-playground/src/ExternalCalls.sol#L72) ```solidity @@ -7446,7 +7472,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 -
33 Found Instances +
34 Found Instances - Found in src/ArbitraryTransferFrom.sol [Line: 9](../tests/contract-playground/src/ArbitraryTransferFrom.sol#L9) @@ -7455,6 +7481,12 @@ State variables that are should be declared immutable to save gas. Add the `immu IERC20 s_token; ``` +- Found in src/EmitAfterExternalCall.sol [Line: 15](../tests/contract-playground/src/EmitAfterExternalCall.sol#L15) + + ```solidity + MaliciousActor s_actor; + ``` + - Found in src/ExternalCalls.sol [Line: 25](../tests/contract-playground/src/ExternalCalls.sol#L25) ```solidity @@ -7668,3 +7700,42 @@ Design the modifier to only contain 1 placeholder statement. If it's not possibl +## L-46: External call is followed by an emit statement + +In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack. + +
4 Found Instances + + +- Found in src/EmitAfterExternalCall.sol [Line: 25](../tests/contract-playground/src/EmitAfterExternalCall.sol#L25) + + Emission happens at: `emit SomeEvent()` + ```solidity + s_actor.hello(); + ``` + +- Found in src/EmitAfterExternalCall.sol [Line: 34](../tests/contract-playground/src/EmitAfterExternalCall.sol#L34) + + Emission happens at: `emit SomeEvent()` + ```solidity + s_actor.hello(); + ``` + +- Found in src/EmitAfterExternalCall.sol [Line: 53](../tests/contract-playground/src/EmitAfterExternalCall.sol#L53) + + Emission happens at: `emit SomeEvent()` + ```solidity + s_actor.hello(); + ``` + +- Found in src/Trump.sol [Line: 315](../tests/contract-playground/src/Trump.sol#L315) + + Emission happens at: `emit ClearToken(tokenAddress, tokens)` + ```solidity + tokens = IERC20(tokenAddress).balanceOf(address(this)); + ``` + +
+ + + diff --git a/reports/report.sarif b/reports/report.sarif index d5bc0058..306c9421 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -1558,6 +1558,28 @@ { "level": "warning", "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 14, + "byteOffset": 66 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/StateChangeAfterExternalCall.sol" + }, + "region": { + "byteLength": 14, + "byteOffset": 66 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -2879,6 +2901,17 @@ { "level": "warning", "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 5, + "byteOffset": 1168 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -6811,6 +6844,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 7, + "byteOffset": 424 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -12298,6 +12342,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 7, + "byteOffset": 373 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -12675,6 +12730,71 @@ "text": "Design the modifier to only contain 1 placeholder statement. If it's not possible, split the logic into multiple modifiers." }, "ruleId": "multiple-placeholders" + }, + { + "level": "note", + "locations": [ + { + "message": { + "text": "Emission happens at: `emit SomeEvent()`" + }, + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 584 + } + } + }, + { + "message": { + "text": "Emission happens at: `emit SomeEvent()`" + }, + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 735 + } + } + }, + { + "message": { + "text": "Emission happens at: `emit SomeEvent()`" + }, + "physicalLocation": { + "artifactLocation": { + "uri": "src/EmitAfterExternalCall.sol" + }, + "region": { + "byteLength": 15, + "byteOffset": 1274 + } + } + }, + { + "message": { + "text": "Emission happens at: `emit ClearToken(tokenAddress, tokens)`" + }, + "physicalLocation": { + "artifactLocation": { + "uri": "src/Trump.sol" + }, + "region": { + "byteLength": 54, + "byteOffset": 10741 + } + } + } + ], + "message": { + "text": "In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack." + }, + "ruleId": "emit-after-external-call" } ], "tool": { diff --git a/reports/templegold-report.md b/reports/templegold-report.md index 80762e09..f7391bd2 100644 --- a/reports/templegold-report.md +++ b/reports/templegold-report.md @@ -46,6 +46,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [L-25: Unused Imports](#l-25-unused-imports) - [L-26: State variable changes but no event is emitted.](#l-26-state-variable-changes-but-no-event-is-emitted) - [L-27: State variable could be declared immutable](#l-27-state-variable-could-be-declared-immutable) + - [L-28: External call is followed by an emit statement](#l-28-external-call-is-followed-by-an-emit-statement) # Summary @@ -199,7 +200,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Category | No. of Issues | | --- | --- | | High | 10 | -| Low | 27 | +| Low | 28 | # High Issues @@ -9217,3 +9218,580 @@ State variables that are should be declared immutable to save gas. Add the `immu +## L-28: External call is followed by an emit statement + +In most cases it is a best practice to perform the emit before making an external call to avoid a potential read only re-entrancy attack. + +
78 Found Instances + + +- Found in contracts/admin/TempleTeamPaymentsFactory.sol [Line: 89](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPaymentsFactory.sol#L89) + + Emission happens at: `emit FundingDeployed( + lastPaidEpoch, + _dests, + _allocations, + address(paymentContract) + )` + ```solidity + paymentContract.initialize(_token); + ``` + +- Found in contracts/admin/TempleTeamPaymentsFactory.sol [Line: 90](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPaymentsFactory.sol#L90) + + Emission happens at: `emit FundingDeployed( + lastPaidEpoch, + _dests, + _allocations, + address(paymentContract) + )` + ```solidity + paymentContract.setAllocations(_dests, _allocations); + ``` + +- Found in contracts/admin/TempleTeamPaymentsFactory.sol [Line: 92](../tests/2024-07-templegold/protocol/contracts/admin/TempleTeamPaymentsFactory.sol#L92) + + Emission happens at: `emit FundingDeployed( + lastPaidEpoch, + _dests, + _allocations, + address(paymentContract) + )` + ```solidity + paymentContract.transferOwnership(msg.sender); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 98](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L98) + + Emission happens at: `emit Mint(msg.sender, amount0, amount1)` + ```solidity + uint balance0 = IERC20(token0).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 99](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L99) + + Emission happens at: `emit Mint(msg.sender, amount0, amount1)` + ```solidity + uint balance1 = IERC20(token1).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 122](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L122) + + Emission happens at: `emit Burn(msg.sender, amount0, amount1, to)` + ```solidity + uint balance0 = IERC20(_token0).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 123](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L123) + + Emission happens at: `emit Burn(msg.sender, amount0, amount1, to)` + ```solidity + uint balance1 = IERC20(_token1).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 133](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L133) + + Emission happens at: `emit Burn(msg.sender, amount0, amount1, to)` + ```solidity + balance0 = IERC20(_token0).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 134](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L134) + + Emission happens at: `emit Burn(msg.sender, amount0, amount1, to)` + ```solidity + balance1 = IERC20(_token1).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 155](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L155) + + Emission happens at: `emit Swap(msg.sender, amount0In, amount1In, amount0Out, amount1Out, to)` + ```solidity + if (data.length > 0) IUniswapV2Callee(to).uniswapV2Call(msg.sender, amount0Out, amount1Out, data); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 156](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L156) + + Emission happens at: `emit Swap(msg.sender, amount0In, amount1In, amount0Out, amount1Out, to)` + ```solidity + balance0 = IERC20(_token0).balanceOf(address(this)); + ``` + +- Found in contracts/amm/TempleUniswapV2Pair.sol [Line: 157](../tests/2024-07-templegold/protocol/contracts/amm/TempleUniswapV2Pair.sol#L157) + + Emission happens at: `emit Swap(msg.sender, amount0In, amount1In, amount0Out, amount1Out, to)` + ```solidity + balance1 = IERC20(_token1).balanceOf(address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 297](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L297) + + Emission happens at: `emit RebalanceUpExit(bptAmountIn, protocolTokenAmountOut, feeAmt)` + ```solidity + amoStaking.withdrawAndUnwrap(bptAmountIn, false, address(_poolHelper)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 300](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L300) + + Emission happens at: `emit RebalanceUpExit(bptAmountIn, protocolTokenAmountOut, feeAmt)` + ```solidity + uint256 protocolTokenAmountOut = _poolHelper.exitPool( + ``` + +- Found in contracts/amo/Ramos.sol [Line: 341](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L341) + + Emission happens at: `emit RebalanceDownExit(bptAmountIn, quoteTokenAmountOut, feeAmt)` + ```solidity + amoStaking.withdrawAndUnwrap(bptAmountIn, false, address(_poolHelper)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 344](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L344) + + Emission happens at: `emit RebalanceDownExit(bptAmountIn, quoteTokenAmountOut, feeAmt)` + ```solidity + uint256 quoteTokenAmountOut = _poolHelper.exitPool( + ``` + +- Found in contracts/amo/Ramos.sol [Line: 382](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L382) + + Emission happens at: `emit RebalanceUpJoin(quoteTokenAmountIn, bptTokensStaked, feeAmt)` + ```solidity + tokenVault.borrowQuoteToken(quoteTokenAmountIn, address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 396](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L396) + + Emission happens at: `emit RebalanceUpJoin(quoteTokenAmountIn, bptTokensStaked, feeAmt)` + ```solidity + uint256 bptTokensStaked = _poolHelper.joinPool( + ``` + +- Found in contracts/amo/Ramos.sol [Line: 427](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L427) + + Emission happens at: `emit RebalanceDownJoin(protocolTokenAmountIn, bptTokensStaked, feeAmt)` + ```solidity + tokenVault.borrowProtocolToken(protocolTokenAmountIn, address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 441](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L441) + + Emission happens at: `emit RebalanceDownJoin(protocolTokenAmountIn, bptTokensStaked, feeAmt)` + ```solidity + uint256 bptTokensStaked = _poolHelper.joinPool( + ``` + +- Found in contracts/amo/Ramos.sol [Line: 481](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L481) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + _tokenVault.borrowProtocolToken(protocolTokenAmount, address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 482](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L482) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + _tokenVault.borrowQuoteToken(quoteTokenAmount, address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 487](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L487) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + uint256 quoteTokenAllowance = quoteToken.allowance(address(this), address(balancerVault)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 489](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L489) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + quoteToken.approve(address(balancerVault), 0); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 496](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L496) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + uint256 bptAmountBefore = bptToken.balanceOf(address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 497](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L497) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + balancerVault.joinPool(balancerPoolId, address(this), address(this), request); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 498](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L498) + + Emission happens at: `emit LiquidityAdded(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + uint256 bptAmountAfter = bptToken.balanceOf(address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 536](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L536) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + uint256 protocolTokenAmountBefore = protocolToken.balanceOf(address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 537](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L537) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + uint256 quoteTokenAmountBefore = quoteToken.balanceOf(address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 539](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L539) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + amoStaking.withdrawAndUnwrap(bptIn, false, address(this)); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 540](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L540) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + balancerVault.exitPool(balancerPoolId, address(this), address(this), request); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 543](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L543) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + protocolTokenAmount = protocolToken.balanceOf(address(this)) - protocolTokenAmountBefore; + ``` + +- Found in contracts/amo/Ramos.sol [Line: 544](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L544) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + quoteTokenAmount = quoteToken.balanceOf(address(this)) - quoteTokenAmountBefore; + ``` + +- Found in contracts/amo/Ramos.sol [Line: 549](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L549) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + _tokenVault.repayProtocolToken(protocolTokenAmount); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 553](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L553) + + Emission happens at: `emit LiquidityRemoved(quoteTokenAmount, protocolTokenAmount, bptIn)` + ```solidity + _tokenVault.repayQuoteToken(quoteTokenAmount); + ``` + +- Found in contracts/amo/Ramos.sol [Line: 572](../tests/2024-07-templegold/protocol/contracts/amo/Ramos.sol#L572) + + Emission happens at: `emit DepositAndStakeBptTokens(amount)` + ```solidity + amoStaking.depositAndStake(amount); + ``` + +- Found in contracts/core/Exposure.sol [Line: 109](../tests/2024-07-templegold/protocol/contracts/core/Exposure.sol#L109) + + Emission happens at: `emit Redeem(address(revalToken), msg.sender, to, amount)` + ```solidity + liquidator.toTemple(amount, to); + ``` + +- Found in contracts/core/MultiOtcOffer.sol [Line: 68](../tests/2024-07-templegold/protocol/contracts/core/MultiOtcOffer.sol#L68) + + Emission happens at: `emit OtcMarketAdded(marketId, address(_otcMarketInfo.userBuyToken), address(_otcMarketInfo.userSellToken))` + ```solidity + uint256 scaleDecimals = marketInfo.offerPricingToken == OfferPricingToken.UserBuyToken + ``` + +- Found in contracts/core/OpsManager.sol [Line: 87](../tests/2024-07-templegold/protocol/contracts/core/OpsManager.sol#L87) + + Emission happens at: `emit CreateVaultInstance(address(vault))` + ```solidity + templeExposure.setMinterState(address(vault), true); + ``` + +- Found in contracts/core/TreasuryFarmingRevenue.sol [Line: 91](../tests/2024-07-templegold/protocol/contracts/core/TreasuryFarmingRevenue.sol#L91) + + Emission happens at: `emit RevenueClaimed(account, unclaimedScaled / SCALING_FACTOR)` + ```solidity + exposure.mint(account, unclaimedScaled / SCALING_FACTOR); + ``` + +- Found in contracts/core/Vault.sol [Line: 179](../tests/2024-07-templegold/protocol/contracts/core/Vault.sol#L179) + + Emission happens at: `emit Deposit(_account, _amount, amountStaked)` + ```solidity + uint256 feePerTempleScaledPerHour = joiningFee.calc(firstPeriodStartTimestamp, periodDuration, address(this)); + ``` + +- Found in contracts/core/Vault.sol [Line: 188](../tests/2024-07-templegold/protocol/contracts/core/Vault.sol#L188) + + Emission happens at: `emit Deposit(_account, _amount, amountStaked)` + ```solidity + templeExposureToken.mint(address(this), _amount); + ``` + +- Found in contracts/deprecated/TempleStaking.sol [Line: 132](../tests/2024-07-templegold/protocol/contracts/deprecated/TempleStaking.sol#L132) + + Emission happens at: `emit StakeCompleted(_staker, _amountTemple, 0)` + ```solidity + OG_TEMPLE.mint(_staker, amountOgTemple); + ``` + +- Found in contracts/deprecated/TempleStaking.sol [Line: 145](../tests/2024-07-templegold/protocol/contracts/deprecated/TempleStaking.sol#L145) + + Emission happens at: `emit UnstakeCompleted(msg.sender, _amountOgTemple)` + ```solidity + require(OG_TEMPLE.allowance(msg.sender, address(this)) >= _amountOgTemple, 'Insufficient OGTemple allowance. Cannot unstake'); + ``` + +- Found in contracts/deprecated/TempleStaking.sol [Line: 150](../tests/2024-07-templegold/protocol/contracts/deprecated/TempleStaking.sol#L150) + + Emission happens at: `emit UnstakeCompleted(msg.sender, _amountOgTemple)` + ```solidity + OG_TEMPLE.burnFrom(msg.sender, _amountOgTemple); + ``` + +- Found in contracts/deprecated/TempleStaking.sol [Line: 152](../tests/2024-07-templegold/protocol/contracts/deprecated/TempleStaking.sol#L152) + + Emission happens at: `emit UnstakeCompleted(msg.sender, _amountOgTemple)` + ```solidity + EXIT_QUEUE.join(msg.sender, unstakeBalanceTemple); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 52](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L52) + + Emission happens at: `emit DsrSet(_newRate)` + ```solidity + _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 110](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L110) + + Emission happens at: `emit AssetBalancesCheckpoint(assetBalances)` + ```solidity + uint256 daiBalance = _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 131](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L131) + + Emission happens at: `emit DaiDeposited(amount)` + ```solidity + _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 134](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L134) + + Emission happens at: `emit DaiDeposited(amount)` + ```solidity + treasuryReservesVault.borrow(daiToken, amount, address(this)); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 145](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L145) + + Emission happens at: `emit DaiWithdrawn(withdrawalAmount)` + ```solidity + uint256 daiAvailable = _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 159](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L159) + + Emission happens at: `emit DaiWithdrawn(daiAvailable)` + ```solidity + uint256 daiAvailable = _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 176](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L176) + + Emission happens at: `emit DaiDeposited(amount)` + ```solidity + uint256 balance = daiToken.balanceOf(address(this)); + ``` + +- Found in contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol [Line: 194](../tests/2024-07-templegold/protocol/contracts/fakes/v2/strategies/DsrBaseStrategyTestnet.sol#L194) + + Emission happens at: `emit DaiWithdrawn(requestedAmount)` + ```solidity + uint256 daiAvailable = _checkpointDaiBalance(daiToken.balanceOf(address(this))); + ``` + +- Found in contracts/governance/ElderElection.sol [Line: 74](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L74) + + Emission happens at: `emit UpdateNomination(discordId, true)` + ```solidity + templars.checkExists(discordId); + ``` + +- Found in contracts/governance/ElderElection.sol [Line: 87](../tests/2024-07-templegold/protocol/contracts/governance/ElderElection.sol#L87) + + Emission happens at: `emit UpdateNomination(discordId, false)` + ```solidity + templars.checkExists(discordId); + ``` + +- Found in contracts/governance/TemplarMetadata.sol [Line: 32](../tests/2024-07-templegold/protocol/contracts/governance/TemplarMetadata.sol#L32) + + Emission happens at: `emit UpdateTempleRole(discordId, _templeRole)` + ```solidity + templars.checkExists(discordId); + ``` + +- Found in contracts/templegold/SpiceAuction.sol [Line: 159](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L159) + + Emission happens at: `emit AuctionStarted(epochId, msg.sender, startTime, endTime, epochAuctionTokenAmount)` + ```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) + + Emission happens at: `emit Deposit(msg.sender, epochId, amount)` + ```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) + + Emission happens at: `emit Deposit(msg.sender, epochId, amount)` + ```solidity + uint256 _bidTokenAmountAfter = IERC20(bidToken).balanceOf(_recipient); + ``` + +- Found in contracts/templegold/SpiceAuction.sol [Line: 260](../tests/2024-07-templegold/protocol/contracts/templegold/SpiceAuction.sol#L260) + + Emission happens at: `emit CommonEventsAndErrors.TokenRecovered(to, token, amount)` + ```solidity + uint256 balance = IERC20(token).balanceOf(address(this)); + ``` + +- Found in contracts/templegold/TempleTeleporter.sol [Line: 54](../tests/2024-07-templegold/protocol/contracts/templegold/TempleTeleporter.sol#L54) + + Emission happens at: `emit TempleTeleported(dstEid, msg.sender, to, amount)` + ```solidity + temple.burnFrom(msg.sender, amount); + ``` + +- Found in contracts/v2/TreasuryReservesVault.sol [Line: 148](../tests/2024-07-templegold/protocol/contracts/v2/TreasuryReservesVault.sol#L148) + + Emission happens at: `emit TpiOracleSet(newTpiOracle)` + ```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) + + Emission happens at: `emit StrategyRemoved(strategy)`, `emit StrategyShutdownCreditAndDebt({ + strategy: strategy, + token: address(_token), + outstandingCredit: credits[_token], + outstandingDebt: _outstandingDebt + })` + ```solidity + _outstandingDebt = borrowTokens[_token].dToken.burnAll(strategy); + ``` + +- Found in contracts/v2/strategies/GnosisStrategy.sol [Line: 89](../tests/2024-07-templegold/protocol/contracts/v2/strategies/GnosisStrategy.sol#L89) + + Emission happens at: `emit Borrow(address(token), amount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/strategies/GnosisStrategy.sol [Line: 104](../tests/2024-07-templegold/protocol/contracts/v2/strategies/GnosisStrategy.sol#L104) + + Emission happens at: `emit Borrow(address(token), borrowedAmount)` + ```solidity + borrowedAmount = _trv.availableForStrategyToBorrow(address(this), token); + ``` + +- Found in contracts/v2/strategies/GnosisStrategy.sol [Line: 105](../tests/2024-07-templegold/protocol/contracts/v2/strategies/GnosisStrategy.sol#L105) + + Emission happens at: `emit Borrow(address(token), borrowedAmount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/strategies/GnosisStrategy.sol [Line: 133](../tests/2024-07-templegold/protocol/contracts/v2/strategies/GnosisStrategy.sol#L133) + + Emission happens at: `emit Repay(address(token), repaidAmount)` + ```solidity + repaidAmount = _trv.repayAll(token, address(this)); + ``` + +- Found in contracts/v2/strategies/RamosStrategy.sol [Line: 92](../tests/2024-07-templegold/protocol/contracts/v2/strategies/RamosStrategy.sol#L92) + + Emission happens at: `emit BorrowToken(address(templeToken), amount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/strategies/RamosStrategy.sol [Line: 107](../tests/2024-07-templegold/protocol/contracts/v2/strategies/RamosStrategy.sol#L107) + + Emission happens at: `emit BorrowToken(address(quoteToken), amount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/strategies/RamosStrategy.sol [Line: 182](../tests/2024-07-templegold/protocol/contracts/v2/strategies/RamosStrategy.sol#L182) + + Emission happens at: `emit AddLiquidity(quoteTokenAmount, protocolTokenAmount, bptTokensStaked)` + ```solidity + ( + ``` + +- Found in contracts/v2/strategies/RamosStrategy.sol [Line: 210](../tests/2024-07-templegold/protocol/contracts/v2/strategies/RamosStrategy.sol#L210) + + Emission happens at: `emit RemoveLiquidity(quoteTokenAmount, protocolTokenAmount, _bptAmount)` + ```solidity + ( + ``` + +- Found in contracts/v2/strategies/TempleTokenBaseStrategy.sol [Line: 62](../tests/2024-07-templegold/protocol/contracts/v2/strategies/TempleTokenBaseStrategy.sol#L62) + + Emission happens at: `emit TempleBurned(amount)` + ```solidity + treasuryReservesVault.borrow(templeToken, amount, address(this)); + ``` + +- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 173](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L173) + + Emission happens at: `emit CollateralRemoved(msg.sender, recipient, amount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 213](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L213) + + Emission happens at: `emit Borrow(msg.sender, recipient, amount)` + ```solidity + circuitBreakerProxy.preCheck( + ``` + +- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 394](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L394) + + Emission happens at: `emit TlcStrategySet(newTlcStrategy, _trv)` + ```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) + + Emission happens at: `emit TlcStrategySet(newTlcStrategy, _trv)` + ```solidity + address _trv = address(tlcStrategy.treasuryReservesVault()); + ``` + +- Found in contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol [Line: 458](../tests/2024-07-templegold/protocol/contracts/v2/templeLineOfCredit/TempleLineOfCredit.sol#L458) + + Emission happens at: `emit CommonEventsAndErrors.TokenRecovered(to, token, amount)` + ```solidity + uint256 bal = templeToken.balanceOf(address(this)); + ``` + +
+ + + diff --git a/tests/contract-playground/src/EmitAfterExternalCall.sol b/tests/contract-playground/src/EmitAfterExternalCall.sol new file mode 100644 index 00000000..59c8e9e1 --- /dev/null +++ b/tests/contract-playground/src/EmitAfterExternalCall.sol @@ -0,0 +1,75 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.19; + +contract MaliciousActor { + // LOGIC inside hello() doesn't matter. So long as it's an external call we don't trust! + function hello() external { + (bool s, ) = msg.sender.call(""); + require(s, "attempt failed"); + } +} + +contract EmitAfterExternalCall { + event SomeEvent(); + + MaliciousActor s_actor; + + constructor(address actor) { + require(actor != address(0)); + s_actor = MaliciousActor(actor); + } + + // BAD + function badSituation1() external { + // Interaction + s_actor.hello(); + + // Effect + emit SomeEvent(); + } + + // BAD + function badSituation2() external { + // Interaction + s_actor.hello(); + + if (msg.sender != address(this)) { + // Effect + emit SomeEvent(); + } + } + + // BAD + function badSituation3() external { + // NOTE: Although this may seem like it's following CEI, because it's inside a loop + //one can imagine that in the second iteration, the effect happens after the interaction + //in the first iteration. + + for (uint256 i = 0; i < 0; ++i) { + // Effect + emit SomeEvent(); + + // Interaction + s_actor.hello(); + } + } + + // GOOD + function goodSituation1() external { + // Effect + emit SomeEvent(); + + // Interaction + s_actor.hello(); + } + + // GOOD + function goodSituation2() external { + if (msg.sender != address(this)) { + s_actor.hello(); + return; + } + + emit SomeEvent(); + } +}