From 2354d7ed73f5444d17f32e2fcd2282052115e381 Mon Sep 17 00:00:00 2001 From: h1kk4 Date: Tue, 6 Jun 2023 15:36:53 +0600 Subject: [PATCH 1/3] check rerurn value of transfer()/transferFrom() --- .../transfer-return-value-not-checked.sol | 37 +++++++++++++++++++ .../transfer-return-value-not-checked.yaml | 23 ++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 solidity/security/transfer-return-value-not-checked.sol create mode 100644 solidity/security/transfer-return-value-not-checked.yaml diff --git a/solidity/security/transfer-return-value-not-checked.sol b/solidity/security/transfer-return-value-not-checked.sol new file mode 100644 index 0000000..5595b37 --- /dev/null +++ b/solidity/security/transfer-return-value-not-checked.sol @@ -0,0 +1,37 @@ +pragma solidity >=0.7.4; + +contract Test{ + function payment() public { + //ok: transfer-return-value-not-checked + bool kek = IERC20(token0).transfer(msg.sender, refund); + } + + function payment2() public { + //ok: transfer-return-value-not-checked + require(IERC20(token0).transfer(msg.sender, refund), "error"); + } + + function payment3() public { + //ruleid: transfer-return-value-not-checked + IERC20(token0).transfer(msg.sender, refund); + } + + + function payment4() public { + //ok: transfer-return-value-not-checked + if(IERC20(token0).transfer(msg.sender, refund)) { + //proccess + } else { + revert("error"); + } + } +} + + +contract Test5 { + IWETH9 weth = address(0); + function sendWETHT(address to, uint256 amount) private { + //ok: transfer-return-value-not-checked + IWETH9(weth).transfer(to, amount); + } +} \ No newline at end of file diff --git a/solidity/security/transfer-return-value-not-checked.yaml b/solidity/security/transfer-return-value-not-checked.yaml new file mode 100644 index 0000000..4544e25 --- /dev/null +++ b/solidity/security/transfer-return-value-not-checked.yaml @@ -0,0 +1,23 @@ +rules: + - id: transfer-return-value-not-checked + metadata: + references: + - https://code4rena.com/reports/2022-03-joyn#h-01-erc20-transferfrom-return-values-not-checked + category: security + tags: + - variable + message: The missing return value check of the ERC20 transfer can result in loss of funds, because some ERC20 tokens indicate a failed transfer by the return value instead of a revert. + languages: + - solidity + severity: WARNING + patterns: + - pattern-either: + - pattern: $VAR.transfer($TO, $AMOUNT); + - pattern: $VAR.transferFrom($FROM, $TO, $AMOUNT); + - pattern-not: $RETURN = $VAR.transfer(...); + - pattern-not: $RETURN = $VAR.transferFrom(...); + - pattern-not-inside: if(...); + - pattern-not-inside: require(...); + - pattern-not-inside: assert(...); + - pattern-not-inside: IWETH9(...); + - pattern-not-inside: IWETH(...); \ No newline at end of file From 77a5d5110e298748f56d90f63a37dcb75c2acc3c Mon Sep 17 00:00:00 2001 From: h1kk4 Date: Tue, 6 Jun 2023 18:33:37 +0600 Subject: [PATCH 2/3] tags + testname fix --- solidity/security/transfer-return-value-not-checked.sol | 2 +- solidity/security/transfer-return-value-not-checked.yaml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/solidity/security/transfer-return-value-not-checked.sol b/solidity/security/transfer-return-value-not-checked.sol index 5595b37..10bda6d 100644 --- a/solidity/security/transfer-return-value-not-checked.sol +++ b/solidity/security/transfer-return-value-not-checked.sol @@ -28,7 +28,7 @@ contract Test{ } -contract Test5 { +contract Test2 { IWETH9 weth = address(0); function sendWETHT(address to, uint256 amount) private { //ok: transfer-return-value-not-checked diff --git a/solidity/security/transfer-return-value-not-checked.yaml b/solidity/security/transfer-return-value-not-checked.yaml index 4544e25..ee00b3a 100644 --- a/solidity/security/transfer-return-value-not-checked.yaml +++ b/solidity/security/transfer-return-value-not-checked.yaml @@ -5,7 +5,8 @@ rules: - https://code4rena.com/reports/2022-03-joyn#h-01-erc20-transferfrom-return-values-not-checked category: security tags: - - variable + - transfer + - transferFrom message: The missing return value check of the ERC20 transfer can result in loss of funds, because some ERC20 tokens indicate a failed transfer by the return value instead of a revert. languages: - solidity From 9954259ac10878da3cce3c43bde92888999bfb16 Mon Sep 17 00:00:00 2001 From: h1kk4 Date: Tue, 6 Jun 2023 19:46:19 +0600 Subject: [PATCH 3/3] fix README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 5c35967..60f6ab8 100644 --- a/README.md +++ b/README.md @@ -113,7 +113,7 @@ no-bidi-characters | Generic | The code must not contain any of Unicode Directio delegatecall-to-arbitrary-address | Generic | An attacker may perform delegatecall() to an arbitrary address. incorrect-use-of-blockhash | Generic | blockhash(block.number) and blockhash(block.number + N) always returns 0. accessible-selfdestruct | Generic | Contract can be destructed by anyone in $FUNC - +transfer-return-value-not-checked | Generic | The return value of transfer/transferFrom functions should be checked ## Gas Optimization Rules Rule ID | Description