Skip to content

External call return value not checked rule #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

External call return value not checked rule #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6481596
+ rule
Frodan Mar 2, 2023
b027c19
+ rule
Frodan Mar 2, 2023
106d562
+ fix
Frodan Mar 3, 2023
fa46ec8
Update external-call-return-value-not-checked.sol
Frodan Mar 3, 2023
e02db9c
Update external-call-return-value-not-checked.yaml
Frodan Mar 3, 2023
e507fd4
Added the call with value case to the external call return value not …
beched Mar 3, 2023
ffd32d3
extend cases with gas limit
h1kk4 Mar 6, 2023
baeb882
update
Frodan Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 98 additions & 0 deletions solidity/external-call-return-value-not-checked.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
pragma solidity ^0.8.17;

contract Test {
function setVars(address payable _contract, uint _num) public payable {

// ok: external-call-return-value-not-checked
(bool success, ) = _contract.delegatecall(
abi.encodeWithSignature("setVars(uint256)", _num)
);
require(success, "Error");

// ok: external-call-return-value-not-checked
(bool success2, ) = _contract.staticcall(
abi.encodeWithSignature("setVars(uint256)", _num)
);
if (!success2) revert();

// // ok: external-call-return-value-not-checked
// (bool success3, bytes memory data) = _contract.staticcall(
// abi.encodeWithSignature("setVars(uint256)", _num)
// );
// if (success3 != 0) revert();

// ok: external-call-return-value-not-checked
(bool success4, bytes memory data) = _contract.call(
abi.encodeWithSignature("setVars(uint256)", _num)
);
if (success4){
revert();
}
// // ok: external-call-return-value-not-checked
// bool success5 = _contract.send(_num);
// if (success5 == 0) revert();

// ok: external-call-return-value-not-checked
(bool success6, ) = _contract.delegatecall(
abi.encodeWithSignature("setVars(uint256)", _num)
);
require(success6 && 1 == 1, "Error");

// ok: external-call-return-value-not-checked
(bool success, bytes memory data) = address(token).call(abi.encodeWithSelector(IERC20.transfer.selector, to, value));
if (!(success && (data.length == 0 || abi.decode(data, (bool))))) revert(RevertMsgExtractor.getRevertMsg(data));

// ok: external-call-return-value-not-checked
(bool sendBackSuccess, ) = payable(msg.sender).call{value: address(this).balance}('');
require(sendBackSuccess, 'Could not send remaining funds to the payer');
}

// function setVars2(address payable _contract, uint _num) public payable {
// // ruleid: external-call-return-value-not-checked
// _contract.call(abi.encodeWithSignature("setVars(uint256)", _num));
// }

function setVars3(address payable _contract, uint _num) public payable {
// ruleid: external-call-return-value-not-checked
(bool success1, bytes memory data) = _contract.call(
abi.encodeWithSignature("setVars(uint256)", _num)
);
}

// function setVars4(address payable _contract, uint _num) public payable {
// // ruleid: external-call-return-value-not-checked
// bool success2 = _contract.send(1 ether);

// }

function setVars5() public payable{
// ok: external-call-return-value-not-checked
(bool success1, bytes memory data) = _contract.call(
abi.encodeWithSignature("setVars(uint256)", _num)
);

return success1 && 1 == 1;
}

function withdrawTaxBalance_() external nonReentrant onlyOwner {
(bool temp1, ) = payable(solidityDevWallet).call{
value: (taxBalance * solidityDevShare) / SHAREDIVISOR
}("");
(bool temp2, ) = payable(frontendDevWallet).call{
value: (taxBalance * frontendDevShare) / SHAREDIVISOR
}("");
(bool temp3, ) = payable(projectLeadWallet).call{
value: (taxBalance * projectLeadShare) / SHAREDIVISOR
}("");
(bool temp4, ) = payable(apeHarambeWallet).call{
value: (taxBalance * apeHarambeShare) / SHAREDIVISOR
}("");

// ok: external-call-return-value-not-checked
(bool temp5, ) = payable(treasuryWallet).call{
value: (taxBalance * treasuryShare) / SHAREDIVISOR
}("");
assert(temp1 && temp2 && temp3 && temp4 && temp5);
taxBalance = 0;
}
}
66 changes: 66 additions & 0 deletions solidity/external-call-return-value-not-checked.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
rules:
- id: external-call-return-value-not-checked
message: Functions call, staticcall, delegatecall, send returns a boolean indicating whether the call completed successfully. Not testing explicitly whether these calls fail may lead to unexpected behavior in the caller contract.
languages:
- solidity
severity: WARNING
metadata:
references:
- https://entethalliance.org/specs/ethtrust-sl/v1/#req-1-check-return
tags:
- ethtrust
- external-call
patterns:
- pattern-either:
- pattern: (bool $SUCC, ) = $ADDR.$METHOD(...);
- pattern: (bool $SUCC, ) = $ADDR.$METHOD{value:$VAL}(...);
- pattern: (bool $SUCC, ) = $ADDR.$METHOD{gas:$GAS}(...);
- pattern: (bool $SUCC, ) = $ADDR.$METHOD{value:$VAL, gas:$GAS}(...);
- pattern: (bool $SUCC, ) = $ADDR.$METHOD{gas:$GAS, value:$VAL}(...);
- pattern: (bool $SUCC, bytes memory $DATA) = $ADDR.$METHOD(...);
- pattern: (bool $SUCC, bytes memory $DATA) = $ADDR.$METHOD{value:$VAL}(...);
- pattern: (bool $SUCC, bytes memory $DATA) = $ADDR.$METHOD{gas:$GAS}(...);
- pattern: (bool $SUCC, bytes memory $DATA) = $ADDR.$METHOD{value:$VAL,
gas:$GAS}(...);
- pattern: (bool $SUCC, bytes memory $DATA) = $ADDR.$METHOD{gas:$GAS,
value:$VAL}(...);
- pattern: $ADDR.$METHOD(...);
- pattern: $ADDR.$METHOD{value:$VAL}(...);
- pattern: $ADDR.$METHOD{gas:$GAS}(...);
- pattern: $ADDR.$METHOD{value:$VAL, gas:$GAS}(...);
- pattern: $ADDR.$METHOD{gas:$GAS, value:$VAL}(...);
- metavariable-regex:
metavariable: $METHOD
regex: ^(call|delegatecall|staticcall|send)$
- pattern-not-inside: |
function $FUNC(...){
...
require(<... $SUCC ...>, ...);
...
}
- pattern-not-inside: |
function $FUNC(...){
...
if (<... $SUCC ...>){
...
}
...
}
- pattern-not-inside: |
function $FUNC(...){
...
return <... $SUCC ...>;
}
- pattern-not-inside: |
function $FUNC(...){
...
$CHECK(..., $SUCC, ...);
...
}
- pattern-not-inside: |
function $FUNC(...){
...
assert(<... $SUCC ...>);
...
}