eth-sri / securify

[DEPRECATED] Security Scanner for Ethereum Smart Contracts
Apache License 2.0
216 stars 50 forks source link

Unrestricted write conflicts #82

Closed hiqua closed 2 years ago

hiqua commented 5 years ago

The tests currently have some conflicts for the "unrestricted write" pattern, e.g.:

{
  "src/test/resources/solidity/reentrancy2.sol:Ownable": {
    "results": {
      "DAO": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "DAOConstantGas": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "LockedEther": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": [
          56
        ]
      },
      "MissingInputValidation": {
        "violations": [],
        "warnings": [
          57,
          83
        ],
        "safe": [],
        "conflicts": []
      },
      "TODAmount": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "TODReceiver": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnhandledException": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnrestrictedEtherFlow": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnrestrictedWrite": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": [
          86
        ]
      }
    },
    "securifyErrors": {
      "errors": []
    }
  },
  "src/test/resources/solidity/reentrancy2.sol:SafeMath": {
    "results": {
      "DAO": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "DAOConstantGas": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "LockedEther": {
        "violations": [],
        "warnings": [
          8
        ],
        "safe": [],
        "conflicts": []
      },
      "MissingInputValidation": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "TODAmount": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "TODReceiver": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnhandledException": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnrestrictedEtherFlow": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "UnrestrictedWrite": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      }
    },
    "securifyErrors": {
      "errors": []
    }
  },
  "src/test/resources/solidity/reentrancy2.sol:Vault": {
    "results": {
      "DAO": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "DAOConstantGas": {
        "violations": [
          148
        ],
        "warnings": [
          132,
          134
        ],
        "safe": [],
        "conflicts": []
      },
      "LockedEther": {
        "violations": [],
        "warnings": [
          96
        ],
        "safe": [],
        "conflicts": []
      },
      "MissingInputValidation": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": []
      },
      "TODAmount": {
        "violations": [],
        "warnings": [
          132,
          134
        ],
        "safe": [
          148
        ],
        "conflicts": []
      },
      "TODReceiver": {
        "violations": [],
        "warnings": [
          134,
          148
        ],
        "safe": [
          132
        ],
        "conflicts": []
      },
      "UnhandledException": {
        "violations": [],
        "warnings": [
          132,
          134,
          148
        ],
        "safe": [],
        "conflicts": []
      },
      "UnrestrictedEtherFlow": {
        "violations": [],
        "warnings": [
          148
        ],
        "safe": [],
        "conflicts": [
          132,
          134
        ]
      },
      "UnrestrictedWrite": {
        "violations": [],
        "warnings": [],
        "safe": [],
        "conflicts": [
          5,
          86,
          120,
          121,
          130,
          145
        ]
      }
    },
    "securifyErrors": {
      "errors": []
    }
  }
}
pragma solidity ^0.4.21;

/**
 * @title SafeMath
 * @dev Math operations with safety checks that throw on error
 */
library SafeMath {

  /**
  * @dev Multiplies two numbers, throws on overflow.
  */
  function mul(uint256 a, uint256 b) internal pure returns (uint256 c) {
    if (a == 0) {
      return 0;
    }
    c = a * b;
    assert(c / a == b);
    return c;
  }

  /**
  * @dev Integer division of two numbers, truncating the quotient.
  */
  function div(uint256 a, uint256 b) internal pure returns (uint256) {
    // assert(b > 0); // Solidity automatically throws when dividing by 0
    // uint256 c = a / b;
    // assert(a == b * c + a % b); // There is no case in which this doesn't hold
    return a / b;
  }

  /**
  * @dev Subtracts two numbers, throws on overflow (i.e. if subtrahend is greater than minuend).
  */
  function sub(uint256 a, uint256 b) internal pure returns (uint256) {
    assert(b <= a);
    return a - b;
  }

  /**
  * @dev Adds two numbers, throws on overflow.
  */
  function add(uint256 a, uint256 b) internal pure returns (uint256 c) {
    c = a + b;
    assert(c >= a);
    return c;
  }
}

/**
 * @title Ownable
 * @dev The Ownable contract has an owner address, and provides basic authorization control
 * functions, this simplifies the implementation of "user permissions".
 */
contract Ownable {
  address public owner;

  event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);

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

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

  /**
   * @dev Allows the current owner to transfer control of the contract to a newOwner.
   * @param newOwner The address to transfer ownership to.
   */
  function transferOwnership(address newOwner) public onlyOwner {
    require(newOwner != address(0));
    emit OwnershipTransferred(owner, newOwner);
    owner = newOwner;
  }

}

/**
 * @title RefundVault
 * @dev This contract is used for storing funds while a crowdsale
 * is in progress.
 */
contract Vault is Ownable {
  using SafeMath for uint256;

  mapping (address => uint256) public deposited;
  address[] fundsOwners;
  address public wallet;

  event Refunded(address beneficiary, uint256 weiAmount);
  event Deposited(address beneficiary, uint256 weiAmount);
  event Released(address beneficiary, uint256 weiAmount);
  event PartialRefund(address beneficiary, uint256 weiAmount);

  /**
   * @param _wallet Vault address
   */
  function Vault(address _wallet) public {
    require(_wallet != address(0));
    wallet = _wallet;
  }

  /**
   * @param beneficiary Investor address
   */
  function deposit(address beneficiary) onlyOwner public payable {
    deposited[beneficiary] = deposited[beneficiary].add(msg.value);
    fundsOwners.push(beneficiary);
    Deposited(beneficiary, msg.value);
  }

  /**
   * @param beneficiary Investor address
   */
  function release(address beneficiary, uint256 overflow) onlyOwner public {
    uint256 amount = deposited[beneficiary].sub(overflow);
    deposited[beneficiary] = 0;

    wallet.transfer(amount);
    if (overflow > 0) {
      beneficiary.transfer(overflow);
      PartialRefund(beneficiary, overflow);
    }
    Released(beneficiary, amount);
  }

  /**
   * @param beneficiary Investor address
   */
  function refund(address beneficiary) onlyOwner public {
    uint256 depositedValue = deposited[beneficiary];
    deposited[beneficiary] = 0;

    Refunded(beneficiary, depositedValue);
    beneficiary.transfer(depositedValue);
  }

  /**
   * refunds all funds on the vault to the corresponding beneficiaries
   * @param indexes list of indexes to look up for in the fundsOwner array to refund
   */
  function refundAll(uint[] indexes) onlyOwner public {
    require(indexes.length <= fundsOwners.length);
    for (uint i = 0; i < indexes.length; i++) {
      refund(fundsOwners[indexes[i]]);
    }
  }
}