SWC智能合约漏洞库

SWC-105/无保护的以太币提款

由于缺少访问控制或访问控制不充分,恶意方可以从合约账户中提取部分或全部以太币。

该错误有时是由于无意中公开了初始化函数引起的。如果错误地命名一个原本用作构造函数的函数, 构造函数代码将以运行时字节代码结尾,并且任何人都可以调用它来重新初始化合约。

CWE漏洞分类

CWE-284:访问控制不当

整改方案

增加访问控制,以便提款只能由授权方或根据智能合约系统的规范来触发。

参考文献

示例合约

tokensalechallenge.sol

/*
 * @source: https://capturetheether.com/challenges/math/token-sale/
 * @author: Steve Marx
 */

pragma solidity ^0.4.21;

contract TokenSaleChallenge {
    mapping(address => uint256) public balanceOf;
    uint256 constant PRICE_PER_TOKEN = 1 ether;

    function TokenSaleChallenge(address _player) public payable {
        require(msg.value == 1 ether);
    }

    function isComplete() public view returns (bool) {
        return address(this).balance < 1 ether;
    }

    function buy(uint256 numTokens) public payable {
        require(msg.value == numTokens * PRICE_PER_TOKEN);

        balanceOf[msg.sender] += numTokens;
    }

    function sell(uint256 numTokens) public {
        require(balanceOf[msg.sender] >= numTokens);

        balanceOf[msg.sender] -= numTokens;
        msg.sender.transfer(numTokens * PRICE_PER_TOKEN);
    }
}

tokensalechallenge.yaml

description: Integer overflow leading into Ether theft
issues:
- id: SWC-101
  count: 3
  locations:
  - bytecode_offsets:
      '0x28bca0703928a8e32ea9dcdc965ef2fc3e5957d467ea62c7df7e29897930512d': [390]
    line_numbers:
      tokensalechallenge.sol: [21]
  - bytecode_offsets:
      '0x28bca0703928a8e32ea9dcdc965ef2fc3e5957d467ea62c7df7e29897930512d': [472]
    line_numbers:
      tokensalechallenge.sol: [23]
  - bytecode_offsets:
      '0x28bca0703928a8e32ea9dcdc965ef2fc3e5957d467ea62c7df7e29897930512d': [672]
    line_numbers:
      tokensalechallenge.sol: [30]
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0x28bca0703928a8e32ea9dcdc965ef2fc3e5957d467ea62c7df7e29897930512d': [693]
    line_numbers:
      tokensalechallenge.sol: [30]

rubixi.sol

pragma solidity ^0.4.22;

contract Rubixi {

        //Declare variables for storage critical to contract
        uint private balance = 0;
        uint private collectedFees = 0;
        uint private feePercent = 10;
        uint private pyramidMultiplier = 300;
        uint private payoutOrder = 0;

        address private creator;

        //Sets creator
        function DynamicPyramid() {
                creator = msg.sender;
        }

        modifier onlyowner {
                if (msg.sender == creator) _;
        }

        struct Participant {
                address etherAddress;
                uint payout;
        }

        Participant[] private participants;

        //Fallback function
        function() {
                init();
        }

        //init function run on fallback
        function init() private {
                //Ensures only tx with value of 1 ether or greater are processed and added to pyramid
                if (msg.value < 1 ether) {
                        collectedFees += msg.value;
                        return;
                }

                uint _fee = feePercent;
                //50% fee rebate on any ether value of 50 or greater
                if (msg.value >= 50 ether) _fee /= 2;

                addPayout(_fee);
        }

        //Function called for valid tx to the contract
        function addPayout(uint _fee) private {
                //Adds new address to participant array
                participants.push(Participant(msg.sender, (msg.value * pyramidMultiplier) / 100));

                //These statements ensure a quicker payout system to later pyramid entrants, so the pyramid has a longer lifespan
                if (participants.length == 10) pyramidMultiplier = 200;
                else if (participants.length == 25) pyramidMultiplier = 150;

                // collect fees and update contract balance
                balance += (msg.value * (100 - _fee)) / 100;
                collectedFees += (msg.value * _fee) / 100;

                //Pays earlier participiants if balance sufficient
                while (balance > participants[payoutOrder].payout) {
                        uint payoutToSend = participants[payoutOrder].payout;
                        participants[payoutOrder].etherAddress.send(payoutToSend);

                        balance -= participants[payoutOrder].payout;
                        payoutOrder += 1;
                }
        }

        //Fee functions for creator
        function collectAllFees() onlyowner {
                if (collectedFees == 0) throw;

                creator.send(collectedFees);
                collectedFees = 0;
        }

        function collectFeesInEther(uint _amt) onlyowner {
                _amt *= 1 ether;
                if (_amt > collectedFees) collectAllFees();

                if (collectedFees == 0) throw;

                creator.send(_amt);
                collectedFees -= _amt;
        }

        function collectPercentOfFees(uint _pcent) onlyowner {
                if (collectedFees == 0 || _pcent > 100) throw;

                uint feesToCollect = collectedFees / 100 * _pcent;
                creator.send(feesToCollect);
                collectedFees -= feesToCollect;
        }

        //Functions for changing variables related to the contract
        function changeOwner(address _owner) onlyowner {
                creator = _owner;
        }

        function changeMultiplier(uint _mult) onlyowner {
                if (_mult > 300 || _mult < 120) throw;

                pyramidMultiplier = _mult;
        }

        function changeFeePercentage(uint _fee) onlyowner {
                if (_fee > 10) throw;

                feePercent = _fee;
        }

        //Functions to provide information to end-user using JSON interface or other interfaces
        function currentMultiplier() constant returns(uint multiplier, string info) {
                multiplier = pyramidMultiplier;
                info = 'This multiplier applies to you as soon as transaction is received, may be lowered to hasten payouts or increased if payouts are fast enough. Due to no float or decimals, multiplier is x100 for a fractional multiplier e.g. 250 is actually a 2.5x multiplier. Capped at 3x max and 1.2x min.';
        }

        function currentFeePercentage() constant returns(uint fee, string info) {
                fee = feePercent;
                info = 'Shown in % form. Fee is halved(50%) for amounts equal or greater than 50 ethers. (Fee may change, but is capped to a maximum of 10%)';
        }

        function currentPyramidBalanceApproximately() constant returns(uint pyramidBalance, string info) {
                pyramidBalance = balance / 1 ether;
                info = 'All balance values are measured in Ethers, note that due to no decimal placing, these values show up as integers only, within the contract itself you will get the exact decimal value you are supposed to';
        }

        function nextPayoutWhenPyramidBalanceTotalsApproximately() constant returns(uint balancePayout) {
                balancePayout = participants[payoutOrder].payout / 1 ether;
        }

        function feesSeperateFromBalanceApproximately() constant returns(uint fees) {
                fees = collectedFees / 1 ether;
        }

        function totalParticipants() constant returns(uint count) {
                count = participants.length;
        }

        function numberOfParticipantsWaitingForPayout() constant returns(uint count) {
                count = participants.length - payoutOrder;
        }

        function participantDetails(uint orderInPyramid) constant returns(address Address, uint Payout) {
                if (orderInPyramid <= participants.length) {
                        Address = participants[orderInPyramid].etherAddress;
                        Payout = participants[orderInPyramid].payout / 1 ether;
                }
        }
}

rubixi.yaml

description: 'Rubixi: Ether can be drained in 2 transactions'
issues:
- id: SWC-105
  count: 3
  locations:
  - bytecode_offsets:
      '0x75b95f0d0703584968adc432f4e136f53a6fdb533990144ae554c5e7918d4db4': [2014]
    line_numbers:
      rubixi.sol: [77]
  - bytecode_offsets:
      '0x75b95f0d0703584968adc432f4e136f53a6fdb533990144ae554c5e7918d4db4': [2644]
    line_numbers:
      rubixi.sol: [87]
  - bytecode_offsets:
      '0x75b95f0d0703584968adc432f4e136f53a6fdb533990144ae554c5e7918d4db4': [1673]
    line_numbers:
      rubixi.sol: [95]

multiowned_not_vulnerable.sol

pragma solidity ^0.4.23;

/**
 * @title MultiOwnable
 */
contract MultiOwnable {
  address public root;
  mapping (address => address) public owners; // owner => parent of owner

  /**
  * @dev The Ownable constructor sets the original `owner` of the contract to the sender
  * account.
  */
  constructor() public {
    root = msg.sender;
    owners[root] = root;
  }

  /**
  * @dev Throws if called by any account other than the owner.
  */
  modifier onlyOwner() {
    require(owners[msg.sender] != 0);
    _;
  }

  /**
  * @dev Adding new owners
  * Note that the "onlyOwner" modifier is missing here.
  */ 
  function newOwner(address _owner) onlyOwner external returns (bool) {
    require(_owner != 0);
    owners[_owner] = msg.sender;
    return true;
  }

  /**
    * @dev Deleting owners
    */
  function deleteOwner(address _owner) onlyOwner external returns (bool) {
    require(owners[_owner] == msg.sender || (owners[_owner] != 0 && msg.sender == root));
    owners[_owner] = 0;
    return true;
  }
}

contract TestContract is MultiOwnable {

  function withdrawAll() onlyOwner {
    msg.sender.transfer(this.balance);
  }

  function() payable {
  }

}

multiowned_not_vulnerable.yaml

description: Ether withdrawal function and correclty named constructor (not vulnerable)
issues:
- id: SWC-105
  count: 0
  locations: []

multiowned_vulnerable.sol

pragma solidity ^0.4.23;

/**
 * @title MultiOwnable
 */
contract MultiOwnable {
  address public root;
  mapping (address => address) public owners; // owner => parent of owner

  /**
  * @dev The Ownable constructor sets the original `owner` of the contract to the sender
  * account.
  */
  constructor() public {
    root = msg.sender;
    owners[root] = root;
  }

  /**
  * @dev Throws if called by any account other than the owner.
  */
  modifier onlyOwner() {
    require(owners[msg.sender] != 0);
    _;
  }

  /**
  * @dev Adding new owners
  * Note that the "onlyOwner" modifier is missing here.
  */ 
  function newOwner(address _owner) external returns (bool) {
    require(_owner != 0);
    owners[_owner] = msg.sender;
    return true;
  }

  /**
    * @dev Deleting owners
    */
  function deleteOwner(address _owner) onlyOwner external returns (bool) {
    require(owners[_owner] == msg.sender || (owners[_owner] != 0 && msg.sender == root));
    owners[_owner] = 0;
    return true;
  }
}

contract TestContract is MultiOwnable {

  function withdrawAll() onlyOwner {
    msg.sender.transfer(this.balance);
  }

  function() payable {
  }

}

multiowned_vulnerable.yaml

description: Ether withdrawal function and correclty named constructor (not vulnerable)
issues:
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0xce9067a8af4d3e76199016c5659d2acdb76482119264e27e59fdf62af40446a9': [789]
    line_numbers:
      multiowned_vulnerable.sol: [50]

simple_ether_drain.sol

pragma solidity ^0.4.22;

contract SimpleEtherDrain {

  function withdrawAllAnyone() {
    msg.sender.transfer(this.balance);
  }

  function () public payable {
  }

}

simple_ether_drain.yaml

description: Anybody can withdraw all Ether from the contract account
issues:
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0x5248a28372021da3b9c50296322a90720938e40e72299d8d93d9c48756ad809a': [156]
    line_numbers:
      simple_ether_drain.sol: [6]

wallet_01_ok.sol

pragma solidity ^0.4.24;

/* User can add pay in and withdraw Ether.
   Nobody can withdraw more Ether than they paid in.
*/

contract Wallet {
    address creator;

    mapping(address => uint256) balances;

    constructor() public {
        creator = msg.sender;
    }

    function deposit() public payable {
        assert(balances[msg.sender] + msg.value > balances[msg.sender]);
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) public {
        require(amount <= balances[msg.sender]);
        msg.sender.transfer(amount);
        balances[msg.sender] -= amount;
    }

    function refund() public {
        msg.sender.transfer(balances[msg.sender]);
        balances[msg.sender] = 0;
    }

    // In an emergency the owner can migrate  allfunds to a different address.

    function migrateTo(address to) public {
        require(creator == msg.sender);
        to.transfer(this.balance);
    }

}

wallet_01_ok.yaml

description: User can withdraw Ether, but not more than they paid in
issues:
- id: SWC-105
  count: 0
  locations: []

wallet_02_refund_nosub.sol

pragma solidity ^0.4.24;

/* User can add pay in and withdraw Ether.
   Unfortunately the developer forgot set the user's balance to 0 when refund() is called.
   An attacker can pay in a small amount of Ether and call refund() repeatedly to empty the contract.
*/

contract Wallet {
    address creator;

    mapping(address => uint256) balances;

    constructor() public {
        creator = msg.sender;
    }

    function deposit() public payable {
        assert(balances[msg.sender] + msg.value > balances[msg.sender]);
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) public {
        require(amount <= balances[msg.sender]);
        msg.sender.transfer(amount);
        balances[msg.sender] -= amount;
    }

    function refund() public {
        msg.sender.transfer(balances[msg.sender]);
    }

    // In an emergency the owner can migrate  allfunds to a different address.

    function migrateTo(address to) public {
        require(creator == msg.sender);
        to.transfer(this.balance);
    }

}

wallet_02_refund_nosub.yaml

description: Attacker can pay in a small amount of Ether and call refund() repeatedly to empty the contract
issues:
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0x06f59ace58eaeefe2d2c5d359776c2b853b1aecb16c2cb1b35b4eacec3d3e7a5': [776]
    line_numbers:
      wallet_02_refund_nosub.sol: [29]

wallet_03_wrong_constructor.sol

pragma solidity ^0.4.24;

/* User can add pay in and withdraw Ether.
   The constructor is wrongly named, so anyone can become 'creator' and withdraw all funds.
*/

contract Wallet {
    address creator;

    mapping(address => uint256) balances;

    function initWallet() public {
        creator = msg.sender;
    }

    function deposit() public payable {
        assert(balances[msg.sender] + msg.value > balances[msg.sender]);
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) public {
        require(amount <= balances[msg.sender]);
        msg.sender.transfer(amount);
        balances[msg.sender] -= amount;
    }

    // In an emergency the owner can migrate  allfunds to a different address.

    function migrateTo(address to) public {
        require(creator == msg.sender);
        to.transfer(this.balance);
    }

}

wallet_03_wrong_constructor.yaml

description: Attacker can become 'creator' by calling initWallet() and withdraw all funds
issues:
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0xed3adab60fdd20b999b0656f703a23055371a57fb8813fe33a5d9a954bcd5484': [705]
    line_numbers:
      wallet_03_wrong_constructor.sol: [31]

wallet_04_confused_sign.sol

pragma solidity ^0.4.24;

/* User can add pay in and withdraw Ether.
   Unfortunatelty, the developer was drunk and used the wrong comparison operator in "withdraw()"
   Anybody can withdraw arbitrary amounts of Ether :()
*/

contract Wallet {
    address creator;

    mapping(address => uint256) balances;

    constructor() public {
        creator = msg.sender;
    }

    function deposit() public payable {
        assert(balances[msg.sender] + msg.value > balances[msg.sender]);
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint256 amount) public {
        require(amount >= balances[msg.sender]);
        msg.sender.transfer(amount);
        balances[msg.sender] -= amount;
    }

    // In an emergency the owner can migrate  allfunds to a different address.

    function migrateTo(address to) public {
        require(creator == msg.sender);
        to.transfer(this.balance);
    }

}

wallet_04_confused_sign.yaml

description: Attacker can withdraw unlimited funds due erroneous comparison operator in withdraw function
issues:
- id: SWC-105
  count: 1
  locations:
  - bytecode_offsets:
      '0xabb778ab80a415887f74d7f545bcb219e03ff41cc152ed99fa9d2122adf87774': [340]
    line_numbers:
      wallet_04_confused_sign.sol: [24]