From 5273e5c31f15a5c3d333fed18094ea944fee9283 Mon Sep 17 00:00:00 2001 From: Tilak Madichetti Date: Mon, 29 Jul 2024 18:58:22 +0530 Subject: [PATCH] Detector: Unchecked `send()` on address (#611) Co-authored-by: Alex Roan --- aderyn_core/src/ast/impls/node/statements.rs | 7 ++ aderyn_core/src/detect/detector.rs | 3 + aderyn_core/src/detect/high/mod.rs | 2 + aderyn_core/src/detect/high/unchecked_send.rs | 113 +++++++++++++++++ .../src/detect/high/weak_randomness.rs | 3 + .../adhoc-sol-files-highs-only-report.json | 1 + reports/report.json | 66 +++++++++- reports/report.md | 117 +++++++++++++----- reports/report.sarif | 97 +++++++++++++++ .../contract-playground/src/UncheckedSend.sol | 31 +++++ 10 files changed, 409 insertions(+), 31 deletions(-) create mode 100644 aderyn_core/src/detect/high/unchecked_send.rs create mode 100644 tests/contract-playground/src/UncheckedSend.sol diff --git a/aderyn_core/src/ast/impls/node/statements.rs b/aderyn_core/src/ast/impls/node/statements.rs index 5ceb70fea..9f1bb74c1 100644 --- a/aderyn_core/src/ast/impls/node/statements.rs +++ b/aderyn_core/src/ast/impls/node/statements.rs @@ -40,9 +40,16 @@ impl Node for ExpressionStatement { fn accept(&self, visitor: &mut impl ASTConstVisitor) -> Result<()> { if visitor.visit_expression_statement(self)? { self.expression.accept(visitor)?; + self.accept_metadata(visitor)?; } visitor.end_visit_expression_statement(self) } + fn accept_metadata(&self, visitor: &mut impl ASTConstVisitor) -> Result<()> { + if let Some(child_id) = self.expression.get_node_id() { + visitor.visit_immediate_children(self.id, vec![child_id])?; + } + Ok(()) + } } impl Node for VariableDeclarationStatement { diff --git a/aderyn_core/src/detect/detector.rs b/aderyn_core/src/detect/detector.rs index 49ad141fa..eef406e72 100644 --- a/aderyn_core/src/detect/detector.rs +++ b/aderyn_core/src/detect/detector.rs @@ -58,6 +58,7 @@ pub fn get_all_issue_detectors() -> Vec> { Box::::default(), Box::::default(), Box::::default(), + Box::::default(), Box::::default(), Box::::default(), Box::::default(), @@ -121,6 +122,7 @@ pub(crate) enum IssueDetectorNamePool { IncorrectCaretOperator, YulReturn, StateVariableShadowing, + UncheckedSend, MisusedBoolean, SendEtherNoChecks, DelegateCallUncheckedAddress, @@ -252,6 +254,7 @@ pub fn request_issue_detector_by_name(detector_name: &str) -> Option { Some(Box::::default()) } + IssueDetectorNamePool::UncheckedSend => Some(Box::::default()), IssueDetectorNamePool::MisusedBoolean => Some(Box::::default()), IssueDetectorNamePool::SendEtherNoChecks => { Some(Box::::default()) diff --git a/aderyn_core/src/detect/high/mod.rs b/aderyn_core/src/detect/high/mod.rs index f5b609722..1e9e9912f 100644 --- a/aderyn_core/src/detect/high/mod.rs +++ b/aderyn_core/src/detect/high/mod.rs @@ -22,6 +22,7 @@ pub(crate) mod state_variable_shadowing; pub(crate) mod storage_array_edit_with_memory; pub(crate) mod tautological_compare; pub(crate) mod unchecked_return; +pub(crate) mod unchecked_send; pub(crate) mod uninitialized_state_variable; pub(crate) mod unprotected_init_function; pub(crate) mod unsafe_casting; @@ -52,6 +53,7 @@ pub use state_variable_shadowing::StateVariableShadowingDetector; pub use storage_array_edit_with_memory::StorageArrayEditWithMemoryDetector; pub use tautological_compare::TautologicalCompareDetector; pub use unchecked_return::UncheckedReturnDetector; +pub use unchecked_send::UncheckedSendDetector; pub use uninitialized_state_variable::UninitializedStateVariableDetector; pub use unprotected_init_function::UnprotectedInitializerDetector; pub use unsafe_casting::UnsafeCastingDetector; diff --git a/aderyn_core/src/detect/high/unchecked_send.rs b/aderyn_core/src/detect/high/unchecked_send.rs new file mode 100644 index 000000000..977272b30 --- /dev/null +++ b/aderyn_core/src/detect/high/unchecked_send.rs @@ -0,0 +1,113 @@ +use std::collections::BTreeMap; +use std::error::Error; + +use crate::ast::{ASTNode, NodeID, NodeType}; + +use crate::capture; +use crate::context::browser::GetImmediateParent; +use crate::detect::detector::IssueDetectorNamePool; +use crate::{ + context::workspace_context::WorkspaceContext, + detect::detector::{IssueDetector, IssueSeverity}, +}; +use eyre::Result; + +#[derive(Default)] +pub struct UncheckedSendDetector { + // 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>, +} + +impl IssueDetector for UncheckedSendDetector { + fn detect(&mut self, context: &WorkspaceContext) -> Result> { + for member_access in context.member_accesses() { + if member_access.member_name == "send" + && member_access + .expression + .type_descriptions() + .is_some_and(|type_desc| { + type_desc + .type_string + .as_ref() + .is_some_and(|type_string| type_string.starts_with("address")) + }) + { + if let Some(ASTNode::FunctionCall(func_call)) = member_access.parent(context) { + if func_call + .parent(context) + .is_some_and(|node| node.node_type() == NodeType::Block) + { + capture!(self, context, func_call); + } + } + } + } + + Ok(!self.found_instances.is_empty()) + } + + fn severity(&self) -> IssueSeverity { + IssueSeverity::High + } + + fn title(&self) -> String { + String::from("Unchecked `bool success` value for send call.") + } + + fn description(&self) -> String { + String::from("The transaction `address(payable?).send(address)` may fail because of reasons like out-of-gas, \ + invalid receipient address or revert from the recipient. Therefore, the boolean returned by this function call must be checked \ + to be `true` in order to verify that the transaction was successful") + } + + fn instances(&self) -> BTreeMap<(String, usize, String), NodeID> { + self.found_instances.clone() + } + + fn name(&self) -> String { + IssueDetectorNamePool::UncheckedSend.to_string() + } +} + +#[cfg(test)] +mod unchecked_send_tests { + use serial_test::serial; + + use crate::detect::{detector::IssueDetector, high::unchecked_send::UncheckedSendDetector}; + + #[test] + #[serial] + fn test_unchecked_send() { + let context = crate::detect::test_utils::load_solidity_source_unit( + "../tests/contract-playground/src/UncheckedSend.sol", + ); + + let mut detector = UncheckedSendDetector::default(); + let found = detector.detect(&context).unwrap(); + + println!("Instances {:#?}", detector.instances()); + + // assert that the detector found an issue + assert!(found); + // assert that the detector found the correct number of instances + assert_eq!(detector.instances().len(), 1); + // assert the severity is high + assert_eq!( + detector.severity(), + crate::detect::detector::IssueSeverity::High + ); + // assert the title is correct + assert_eq!( + detector.title(), + String::from("Unchecked `bool success` value for send call.") + ); + // assert the description is correct + assert_eq!( + detector.description(), + String::from("The transaction `address(payable?).send(address)` may fail because of reasons like out-of-gas, \ + invalid receipient address or revert from the recipient. Therefore, the boolean returned by this function call must be checked \ + to be `true` in order to verify that the transaction was successful") + ); + } +} diff --git a/aderyn_core/src/detect/high/weak_randomness.rs b/aderyn_core/src/detect/high/weak_randomness.rs index 079fd2cc2..16280ee4a 100644 --- a/aderyn_core/src/detect/high/weak_randomness.rs +++ b/aderyn_core/src/detect/high/weak_randomness.rs @@ -160,9 +160,12 @@ fn check_operand(operand: &Expression) -> bool { #[cfg(test)] mod weak_randomness_detector_tests { + use serial_test::serial; + use crate::detect::{detector::IssueDetector, high::weak_randomness::WeakRandomnessDetector}; #[test] + #[serial] fn test_weak_randomness_detector() { let context = crate::detect::test_utils::load_solidity_source_unit( "../tests/contract-playground/src/WeakRandomness.sol", diff --git a/reports/adhoc-sol-files-highs-only-report.json b/reports/adhoc-sol-files-highs-only-report.json index 5b6f13587..3f591e527 100644 --- a/reports/adhoc-sol-files-highs-only-report.json +++ b/reports/adhoc-sol-files-highs-only-report.json @@ -181,6 +181,7 @@ "incorrect-caret-operator", "yul-return", "state-variable-shadowing", + "unchecked-send", "misused-boolean", "send-ether-no-checks", "delegate-call-unchecked-address", diff --git a/reports/report.json b/reports/report.json index 1e7161940..caa2ecc85 100644 --- a/reports/report.json +++ b/reports/report.json @@ -1,7 +1,7 @@ { "files_summary": { - "total_source_units": 67, - "total_sloc": 1904 + "total_source_units": 68, + "total_sloc": 1922 }, "files_details": { "files_details": [ @@ -165,6 +165,10 @@ "file_path": "src/UncheckedReturn.sol", "n_sloc": 33 }, + { + "file_path": "src/UncheckedSend.sol", + "n_sloc": 18 + }, { "file_path": "src/UninitializedStateVariable.sol", "n_sloc": 29 @@ -276,7 +280,7 @@ ] }, "issue_count": { - "high": 29, + "high": 30, "low": 23 }, "high_issues": { @@ -1380,6 +1384,19 @@ } ] }, + { + "title": "Unchecked `bool success` value for send call.", + "description": "The transaction `address(payable?).send(address)` may fail because of reasons like out-of-gas, invalid receipient address or revert from the recipient. Therefore, the boolean returned by this function call must be checked to be `true` in order to verify that the transaction was successful", + "detector_name": "unchecked-send", + "instances": [ + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 24, + "src": "815:22", + "src_char": "815:22" + } + ] + }, { "title": "Misused boolean with logical operators", "description": "The patterns `if (… || true)` and `if (.. && false)` will always evaluate to true and false respectively.", @@ -1476,6 +1493,30 @@ "src": "1795:5", "src_char": "1795:5" }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 6, + "src": "85:246", + "src_char": "85:246" + }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 12, + "src": "337:190", + "src_char": "337:190" + }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 17, + "src": "533:184", + "src_char": "533:184" + }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 22, + "src": "723:186", + "src_char": "723:186" + }, { "contract_path": "src/UninitializedStateVariable.sol", "line_no": 17, @@ -1938,6 +1979,12 @@ "src": "32:23", "src_char": "32:23" }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 2, + "src": "32:23", + "src_char": "32:23" + }, { "contract_path": "src/UsingSelfdestruct.sol", "line_no": 2, @@ -2932,6 +2979,12 @@ "src": "1795:5", "src_char": "1795:5" }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 27, + "src": "915:65", + "src_char": "915:65" + }, { "contract_path": "src/auditor_mode/PublicFunctionsWithoutSenderCheck.sol", "line_no": 11, @@ -3197,6 +3250,12 @@ "line_no": 17, "src": "388:11", "src_char": "388:11" + }, + { + "contract_path": "src/UncheckedSend.sol", + "line_no": 27, + "src": "915:65", + "src_char": "915:65" } ] }, @@ -3466,6 +3525,7 @@ "incorrect-caret-operator", "yul-return", "state-variable-shadowing", + "unchecked-send", "misused-boolean", "send-ether-no-checks", "delegate-call-unchecked-address", diff --git a/reports/report.md b/reports/report.md index d2cd2eeca..dba746648 100644 --- a/reports/report.md +++ b/reports/report.md @@ -27,16 +27,17 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati - [H-17: Incorrect use of caret operator on a non hexadcimal constant](#h-17-incorrect-use-of-caret-operator-on-a-non-hexadcimal-constant) - [H-18: Yul block contains `return` function call.](#h-18-yul-block-contains-return-function-call) - [H-19: High Issue Title](#h-19-high-issue-title) - - [H-20: Misused boolean with logical operators](#h-20-misused-boolean-with-logical-operators) - - [H-21: Sending native Eth is not protected from these functions.](#h-21-sending-native-eth-is-not-protected-from-these-functions) - - [H-22: Delegatecall made by the function without checks on any adress.](#h-22-delegatecall-made-by-the-function-without-checks-on-any-adress) - - [H-23: Tautological comparison.](#h-23-tautological-comparison) - - [H-24: RTLO character detected in file. \u{202e}](#h-24-rtlo-character-detected-in-file-u202e) - - [H-25: Return value of the function call is not checked.](#h-25-return-value-of-the-function-call-is-not-checked) - - [H-26: Dangerous unary operator found in assignment.](#h-26-dangerous-unary-operator-found-in-assignment) - - [H-27: Weak Randomness](#h-27-weak-randomness) - - [H-28: Usage of variable before declaration.](#h-28-usage-of-variable-before-declaration) - - [H-29: Deletion from a nested mappping.](#h-29-deletion-from-a-nested-mappping) + - [H-20: Unchecked `bool success` value for send call.](#h-20-unchecked-bool-success-value-for-send-call) + - [H-21: Misused boolean with logical operators](#h-21-misused-boolean-with-logical-operators) + - [H-22: Sending native Eth is not protected from these functions.](#h-22-sending-native-eth-is-not-protected-from-these-functions) + - [H-23: Delegatecall made by the function without checks on any adress.](#h-23-delegatecall-made-by-the-function-without-checks-on-any-adress) + - [H-24: Tautological comparison.](#h-24-tautological-comparison) + - [H-25: RTLO character detected in file. \u{202e}](#h-25-rtlo-character-detected-in-file-u202e) + - [H-26: Return value of the function call is not checked.](#h-26-return-value-of-the-function-call-is-not-checked) + - [H-27: Dangerous unary operator found in assignment.](#h-27-dangerous-unary-operator-found-in-assignment) + - [H-28: Weak Randomness](#h-28-weak-randomness) + - [H-29: Usage of variable before declaration.](#h-29-usage-of-variable-before-declaration) + - [H-30: Deletion from a nested mappping.](#h-30-deletion-from-a-nested-mappping) - [Low Issues](#low-issues) - [L-1: Centralization Risk for trusted owners](#l-1-centralization-risk-for-trusted-owners) - [L-2: Solmate's SafeTransferLib does not check for token contract's existence](#l-2-solmates-safetransferlib-does-not-check-for-token-contracts-existence) @@ -69,8 +70,8 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | Key | Value | | --- | --- | -| .sol Files | 67 | -| Total nSLOC | 1904 | +| .sol Files | 68 | +| Total nSLOC | 1922 | ## Files Details @@ -117,6 +118,7 @@ This report was generated by [Aderyn](https://github.com/Cyfrin/aderyn), a stati | src/TautologicalCompare.sol | 17 | | src/TestERC20.sol | 62 | | src/UncheckedReturn.sol | 33 | +| src/UncheckedSend.sol | 18 | | src/UninitializedStateVariable.sol | 29 | | src/UnprotectedInitialize.sol | 25 | | src/UnsafeERC721Mint.sol | 18 | @@ -144,14 +146,14 @@ 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** | **1904** | +| **Total** | **1922** | ## Issue Summary | Category | No. of Issues | | --- | --- | -| High | 29 | +| High | 30 | | Low | 23 | @@ -1334,7 +1336,24 @@ Description of the high issue. -## H-20: Misused boolean with logical operators +## H-20: Unchecked `bool success` value for send call. + +The transaction `address(payable?).send(address)` may fail because of reasons like out-of-gas, invalid receipient address or revert from the recipient. Therefore, the boolean returned by this function call must be checked to be `true` in order to verify that the transaction was successful + +
1 Found Instances + + +- Found in src/UncheckedSend.sol [Line: 24](../tests/contract-playground/src/UncheckedSend.sol#L24) + + ```solidity + recipient.send(amount); // parent of Send FunctionCall is Block (return value is unused) + ``` + +
+ + + +## H-21: Misused boolean with logical operators The patterns `if (… || true)` and `if (.. && false)` will always evaluate to true and false respectively. @@ -1405,11 +1424,11 @@ The patterns `if (… || true)` and `if (.. && false)` will always evaluate to t -## H-21: Sending native Eth is not protected from these functions. +## H-22: Sending native Eth is not protected from these functions. Introduce checks for `msg.sender` in the function -
5 Found Instances +
9 Found Instances - Found in src/CallGraphTests.sol [Line: 38](../tests/contract-playground/src/CallGraphTests.sol#L38) @@ -1436,6 +1455,30 @@ Introduce checks for `msg.sender` in the function function func1(address x) external mod1(x) { ``` +- Found in src/UncheckedSend.sol [Line: 6](../tests/contract-playground/src/UncheckedSend.sol#L6) + + ```solidity + function send1(address payable recipient, uint256 amount) external { + ``` + +- Found in src/UncheckedSend.sol [Line: 12](../tests/contract-playground/src/UncheckedSend.sol#L12) + + ```solidity + function send2(address payable recipient, uint256 amount) external { + ``` + +- Found in src/UncheckedSend.sol [Line: 17](../tests/contract-playground/src/UncheckedSend.sol#L17) + + ```solidity + function send3(address payable recipient, uint256 amount) external returns(bool) { + ``` + +- Found in src/UncheckedSend.sol [Line: 22](../tests/contract-playground/src/UncheckedSend.sol#L22) + + ```solidity + function send4(address payable recipient, uint256 amount) external { + ``` + - Found in src/UninitializedStateVariable.sol [Line: 17](../tests/contract-playground/src/UninitializedStateVariable.sol#L17) ```solidity @@ -1446,7 +1489,7 @@ Introduce checks for `msg.sender` in the function -## H-22: Delegatecall made by the function without checks on any adress. +## H-23: Delegatecall made by the function without checks on any adress. Introduce checks on the address @@ -1475,7 +1518,7 @@ Introduce checks on the address -## H-23: Tautological comparison. +## H-24: Tautological comparison. The left hand side and the right hand side of the binary operation has the same value. This makes the condition always true or always false. @@ -1510,7 +1553,7 @@ The left hand side and the right hand side of the binary operation has the same -## H-24: RTLO character detected in file. \u{202e} +## H-25: RTLO character detected in file. \u{202e} Right to left override character may be misledaing and cause potential attacks by visually misordering method arguments! @@ -1527,7 +1570,7 @@ Right to left override character may be misledaing and cause potential attacks b -## H-25: Return value of the function call is not checked. +## H-26: Return value of the function call is not checked. Function returns a value but it is ignored. @@ -1550,7 +1593,7 @@ Function returns a value but it is ignored. -## H-26: Dangerous unary operator found in assignment. +## H-27: Dangerous unary operator found in assignment. Potentially mistakened `=+` for `+=` or `=-` for `-=`. Please include a space in between. @@ -1573,7 +1616,7 @@ Potentially mistakened `=+` for `+=` or `=-` for `-=`. Please include a space in -## H-27: Weak Randomness +## H-28: Weak Randomness The use of keccak256 hash functions on predictable values like block.timestamp, block.number, or similar data, including modulo operations on these values, should be avoided for generating randomness, as they are easily predictable and manipulable. The `PREVRANDAO` opcode also should not be used as a source of randomness. Instead, utilize Chainlink VRF for cryptographically secure and provably random values to ensure protocol integrity. @@ -1638,7 +1681,7 @@ The use of keccak256 hash functions on predictable values like block.timestamp, -## H-28: Usage of variable before declaration. +## H-29: Usage of variable before declaration. This is a bad practice that may lead to unintended consequences. Please declare the variable before using it. @@ -1655,7 +1698,7 @@ This is a bad practice that may lead to unintended consequences. Please declare -## H-29: Deletion from a nested mappping. +## H-30: Deletion from a nested mappping. A deletion in a structure containing a mapping will not delete the mapping. The remaining data may be used to compromise the contract. @@ -1904,7 +1947,7 @@ ERC20 functions may not behave as expected. For example: return values are not a Consider using a specific version of Solidity in your contracts instead of a wide version. For example, instead of `pragma solidity ^0.8.0;`, use `pragma solidity 0.8.0;` -
15 Found Instances +
16 Found Instances - Found in src/ContractWithTodo.sol [Line: 2](../tests/contract-playground/src/ContractWithTodo.sol#L2) @@ -1955,6 +1998,12 @@ Consider using a specific version of Solidity in your contracts instead of a wid pragma solidity ^0.4.0; ``` +- Found in src/UncheckedSend.sol [Line: 2](../tests/contract-playground/src/UncheckedSend.sol#L2) + + ```solidity + pragma solidity ^0.7.0; + ``` + - Found in src/UsingSelfdestruct.sol [Line: 2](../tests/contract-playground/src/UsingSelfdestruct.sol#L2) ```solidity @@ -2878,7 +2927,7 @@ Solc compiler version 0.8.20 switches the default target EVM version to Shanghai Consider removing empty blocks. -
25 Found Instances +
26 Found Instances - Found in src/AdminContract.sol [Line: 14](../tests/contract-playground/src/AdminContract.sol#L14) @@ -2977,6 +3026,12 @@ Consider removing empty blocks. function func1(address x) external mod1(x) { ``` +- Found in src/UncheckedSend.sol [Line: 27](../tests/contract-playground/src/UncheckedSend.sol#L27) + + ```solidity + function doSomething(bool success) internal pure { + ``` + - Found in src/auditor_mode/PublicFunctionsWithoutSenderCheck.sol [Line: 11](../tests/contract-playground/src/auditor_mode/PublicFunctionsWithoutSenderCheck.sol#L11) ```solidity @@ -3182,7 +3237,7 @@ Use `e` notation, for example: `1e18`, instead of its full numeric value. Instead of separating the logic into a separate function, consider inlining the logic into the calling function. This can reduce the number of function calls and improve readability. -
11 Found Instances +
12 Found Instances - Found in src/CallGraphTests.sol [Line: 6](../tests/contract-playground/src/CallGraphTests.sol#L6) @@ -3251,6 +3306,12 @@ Instead of separating the logic into a separate function, consider inlining the function editStorage(uint[1] storage arr) internal { ``` +- Found in src/UncheckedSend.sol [Line: 27](../tests/contract-playground/src/UncheckedSend.sol#L27) + + ```solidity + function doSomething(bool success) internal pure { + ``` +
diff --git a/reports/report.sarif b/reports/report.sarif index 914985a7e..ddc8f5e5d 100644 --- a/reports/report.sarif +++ b/reports/report.sarif @@ -1946,6 +1946,26 @@ }, "ruleId": "state-variable-shadowing" }, + { + "level": "warning", + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 22, + "byteOffset": 815 + } + } + } + ], + "message": { + "text": "The transaction `address(payable?).send(address)` may fail because of reasons like out-of-gas, invalid receipient address or revert from the recipient. Therefore, the boolean returned by this function call must be checked to be `true` in order to verify that the transaction was successful" + }, + "ruleId": "unchecked-send" + }, { "level": "warning", "locations": [ @@ -2112,6 +2132,50 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 246, + "byteOffset": 85 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 190, + "byteOffset": 337 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 184, + "byteOffset": 533 + } + } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 186, + "byteOffset": 723 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -2898,6 +2962,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 23, + "byteOffset": 32 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -4682,6 +4757,17 @@ } } }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 65, + "byteOffset": 915 + } + } + }, { "physicalLocation": { "artifactLocation": { @@ -5161,6 +5247,17 @@ "byteOffset": 388 } } + }, + { + "physicalLocation": { + "artifactLocation": { + "uri": "src/UncheckedSend.sol" + }, + "region": { + "byteLength": 65, + "byteOffset": 915 + } + } } ], "message": { diff --git a/tests/contract-playground/src/UncheckedSend.sol b/tests/contract-playground/src/UncheckedSend.sol new file mode 100644 index 000000000..6822b75fa --- /dev/null +++ b/tests/contract-playground/src/UncheckedSend.sol @@ -0,0 +1,31 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.7.0; + +contract SendExample { + + function send1(address payable recipient, uint256 amount) external { + // GOOD + bool success = recipient.send(amount); // parent of Send FunctionCall is VariableDeclarationStatement + require(success, "Send successful!"); + } + + function send2(address payable recipient, uint256 amount) external { + // GOOD + doSomething(recipient.send(amount)); // parent of Send FunctionCall is another FunctionCall + } + + function send3(address payable recipient, uint256 amount) external returns(bool) { + // GOOD + return recipient.send(amount); // parent of Send FunctionCall is return + } + + function send4(address payable recipient, uint256 amount) external { + // BAD + recipient.send(amount); // parent of Send FunctionCall is Block (return value is unused) + } + + function doSomething(bool success) internal pure { + + } + +}