diff --git a/README.md b/README.md index 5c35967..153b4b4 100644 --- a/README.md +++ b/README.md @@ -113,6 +113,8 @@ 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 +unsafe-transferfrom-with-ierc20 | Generic | Use OpenZeppelin's SafeERC20's safeTransferFrom() instead of transferFrom() +unsafe-transfer-with-ierc20 | Generic | Use OpenZeppelin's SafeERC20's safeTransfer() instead of transfer() ## Gas Optimization Rules diff --git a/solidity/security/unsafe-transfer-with-ierc20.sol b/solidity/security/unsafe-transfer-with-ierc20.sol new file mode 100644 index 0000000..addfdaa --- /dev/null +++ b/solidity/security/unsafe-transfer-with-ierc20.sol @@ -0,0 +1,37 @@ +pragma solidity >=0.7.4; + +contract Test { + function payment() public { + //ruleid: unsafe-transfer-with-ierc20 + IERC20(token0).transfer(msg.sender, refund); + } + + function payment2() public { + //ruleid: unsafe-transfer-with-ierc20 + require(IERC20(token0).transfer(msg.sender, refund),"error"); + } + + function payment3() public { + //ruleid: unsafe-transfer-with-ierc20 + bool success = IERC20(token0).transfer(msg.sender, refund); + } +} + +contract Test2 { + IERC20 token0 = address(0); + function payment() public { + //ruleid: unsafe-transfer-with-ierc20 + token0.transfer(msg.sender, refund); + } +} + +contract Test3 { + IERC20 token0; + constructor() { + token0 = address(0); + } + function payment() public { + //ruleid: unsafe-transfer-with-ierc20 + token0.transfer(msg.sender, refund); + } +} \ No newline at end of file diff --git a/solidity/security/unsafe-transfer-with-ierc20.yaml b/solidity/security/unsafe-transfer-with-ierc20.yaml new file mode 100644 index 0000000..121c26c --- /dev/null +++ b/solidity/security/unsafe-transfer-with-ierc20.yaml @@ -0,0 +1,45 @@ +rules: + - id: unsafe-transfer-with-ierc20 + metadata: + references: + - https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca + category: security + tags: + - IERC20 + - transfer + message: The majority of code that accepts ERC20 tokens still accepts some tokens even when they do not correctly implement the standard. Using IERC20 interface to work with that kind of token may lead to inablility to transfer funds from smart contract. + Use OpenZeppelin's SafeERC20's safeTransfer() instead of transfer() to prevent this issue. + languages: + - solidity + severity: WARNING + patterns: + - pattern: | + $VAR.transfer($TO, $AMOUNT); + - pattern-either: + - pattern-inside: | + contract $C { + ... + IERC20 $VAR = ...; + ... + function $F(...) { + ... + $VAR.transfer($TO, $AMOUNT); + ... + } + ... + } + - pattern-inside: | + contract $C { + ... + IERC20 $VAR; + ... + function $F(...) { + ... + $VAR.transfer($TO, $AMOUNT); + ... + } + ... + } + - pattern-inside: IERC20(...); + + \ No newline at end of file diff --git a/solidity/security/unsafe-transferfrom-with-ierc20.sol b/solidity/security/unsafe-transferfrom-with-ierc20.sol new file mode 100644 index 0000000..1b722fd --- /dev/null +++ b/solidity/security/unsafe-transferfrom-with-ierc20.sol @@ -0,0 +1,37 @@ +pragma solidity >=0.7.4; + +contract Test { + function payment() public { + //ruleid: unsafe-transferfrom-with-ierc20 + IERC20(token0).transferFrom(msg.sender, address(this), refund); + } + + function payment2() public { + //ruleid: unsafe-transferfrom-with-ierc20 + require(IERC20(token0).transferFrom(msg.sender, address(this), refund),"error"); + } + + function payment3() public { + //ruleid: unsafe-transferfrom-with-ierc20 + bool success = IERC20(token0).transferFrom(msg.sender, address(this), refund); + } +} + +contract Test2 { + IERC20 token0 = address(0); + function payment() public { + //ruleid: unsafe-transferfrom-with-ierc20 + token0.transferFrom(msg.sender, address(this), refund); + } +} + +contract Test3 { + IERC20 token0; + constructor() { + token0 = address(0); + } + function payment() public { + //ruleid: unsafe-transferfrom-with-ierc20 + token0.transferFrom(msg.sender, address(this), refund); + } +} \ No newline at end of file diff --git a/solidity/security/unsafe-transferfrom-with-ierc20.yaml b/solidity/security/unsafe-transferfrom-with-ierc20.yaml new file mode 100644 index 0000000..65d1f51 --- /dev/null +++ b/solidity/security/unsafe-transferfrom-with-ierc20.yaml @@ -0,0 +1,43 @@ +rules: + - id: unsafe-transferfrom-with-ierc20 + metadata: + references: + - https://medium.com/coinmonks/missing-return-value-bug-at-least-130-tokens-affected-d67bf08521ca + category: security + tags: + - IERC20 + - transferFrom + message: The majority of code that accepts ERC20 tokens still accepts some tokens even when they do not correctly implement the standard. Using IERC20 interface to work with that kind of token may lead to inablility to transfer funds from smart contract. + Use OpenZeppelin's SafeERC20's safeTransferFrom() instead of transferFrom() to prevent this issue. + languages: + - solidity + severity: WARNING + patterns: + - pattern: | + $VAR.transferFrom($FROM, $TO, $AMOUNT); + - pattern-either: + - pattern-inside: | + contract $C { + ... + IERC20 $VAR = ...; + ... + function $F(...) { + ... + $VAR.transferFrom($FROM, $TO, $AMOUNT); + ... + } + ... + } + - pattern-inside: | + contract $C { + ... + IERC20 $VAR; + ... + function $F(...) { + ... + $VAR.transferFrom($FROM, $TO, $AMOUNT); + ... + } + ... + } + - pattern-inside: IERC20(...); \ No newline at end of file