Title
Code With No Effects
Relationships
Description
In Solidity, it's possible to write code that does not produce the intended effects. Currently, the solidity compiler will not return a warning for effect-free code. This can lead to the introduction of "dead" code that does not properly perform an intended action.
For example, it's easy to miss the trailing parentheses in msg.sender.call.value(address(this).balance)("");
, which could lead to a function proceeding without transferring funds to msg.sender
. Although, this should be avoided by checking the return value of the call.
Remediation
It's important to carefully ensure that your contract works as intended. Write unit tests to verify correct behaviour of the code.
References
- Issue on Solidity's Github - raise an error when a statement can never have side-effects
- Issue on Solidity's Github - msg.sender.call.value(address(this).balance); should produce a warning
Samples
deposit_box.sol
pragma solidity ^0.5.0;
contract DepositBox {
mapping(address => uint) balance;
// Accept deposit
function deposit(uint amount) public payable {
require(msg.value == amount, 'incorrect amount');
// Should update user balance
balance[msg.sender] == amount;
}
}
deposit_box_fixed.sol
pragma solidity ^0.5.0;
contract DepositBox {
mapping(address => uint) balance;
// Accept deposit
function deposit(uint amount) public payable {
require(msg.value == amount, 'incorrect amount');
// Should update user balance
balance[msg.sender] = amount;
}
}
wallet.sol
/*
* @author: Kaden Zipfel
*/
pragma solidity ^0.5.0;
contract Wallet {
mapping(address => uint) balance;
// Deposit funds in contract
function deposit(uint amount) public payable {
require(msg.value == amount, 'msg.value must be equal to amount');
balance[msg.sender] = amount;
}
// Withdraw funds from contract
function withdraw(uint amount) public {
require(amount <= balance[msg.sender], 'amount must be less than balance');
uint previousBalance = balance[msg.sender];
balance[msg.sender] = previousBalance - amount;
// Attempt to send amount from the contract to msg.sender
msg.sender.call.value(amount);
}
}
wallet_fixed.sol
/*
* @author: Kaden Zipfel
*/
pragma solidity ^0.5.0;
contract Wallet {
mapping(address => uint) balance;
// Deposit funds in contract
function deposit(uint amount) public payable {
require(msg.value == amount, 'msg.value must be equal to amount');
balance[msg.sender] = amount;
}
// Withdraw funds from contract
function withdraw(uint amount) public {
require(amount <= balance[msg.sender], 'amount must be less than balance');
uint previousBalance = balance[msg.sender];
balance[msg.sender] = previousBalance - amount;
// Attempt to send amount from the contract to msg.sender
(bool success, ) = msg.sender.call.value(amount)("");
require(success, 'transfer failed');
}
}