Skip to content
This repository has been archived by the owner on Sep 22, 2022. It is now read-only.

Commit

Permalink
Reviewed and retested with new changes
Browse files Browse the repository at this point in the history
  • Loading branch information
bokkypoobah committed Nov 15, 2017
1 parent 38f87d3 commit 862367c
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 345 deletions.
12 changes: 7 additions & 5 deletions audit/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@

Bok Consulting Pty Ltd was commissioned to perform an audit on the Oracles Network's presale Ethereum smart contract.

This audit has been conducted on Oracles Network's source code in commit
[5bc4391](https://github.com/oraclesorg/oracles-presale/commit/5bc439115ecebb0a52cfe9305f00f89756c5a90a).
This audit has been conducted on Oracles Network's source code in commits
[5bc4391](https://github.com/oraclesorg/oracles-presale/commit/5bc439115ecebb0a52cfe9305f00f89756c5a90a) and
[565e090](https://github.com/oraclesorg/oracles-presale/commit/565e090b0a2a37f36aedf3c5585cb03f2cfb4b33).

No potential vulnerabilities have been identified in the presale contract.

Expand Down Expand Up @@ -50,14 +51,15 @@ No tokens are generated for these contributions.

* **LOW IMPORTANCE** Use the OpenZeppelin *Claimable* contract instead of the *Ownable* contract to provide more safety during the
ownership transfer process

* [x] Updated in [565e090](https://github.com/oraclesorg/oracles-presale/commit/565e090b0a2a37f36aedf3c5585cb03f2cfb4b33)
* **LOW IMPORTANCE** The variable `PresaleOracles.rate` is unused and can be removed

* [x] Updated in [565e090](https://github.com/oraclesorg/oracles-presale/commit/565e090b0a2a37f36aedf3c5585cb03f2cfb4b33)
* **LOW IMPORTANCE** Consider adding the logging of an event for each contribution, like `Contribution(address investor, uint investorAmount, uint investorTotal, uint totalAmount)`.
This data can then be easily extracted using a script, if there are many individual contributions

* [x] Updated in [565e090](https://github.com/oraclesorg/oracles-presale/commit/565e090b0a2a37f36aedf3c5585cb03f2cfb4b33)
* **LOW IMPORTANCE** `BasicToken` can be replaced with the `ERC20Basic` interface in `PresaleOracles.claimTokens(...)`, and the `BasicToken`
contract source code can be removed
* [x] Updated in [565e090](https://github.com/oraclesorg/oracles-presale/commit/565e090b0a2a37f36aedf3c5585cb03f2cfb4b33)

<br />

Expand Down
83 changes: 41 additions & 42 deletions audit/code-review/PresaleOracles_flat.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,61 +101,59 @@ contract Ownable {
}

// BK Ok
contract ERC20Basic {
// BK Ok
uint256 public totalSupply;
contract Claimable is Ownable {
// BK Ok
function balanceOf(address who) public constant returns (uint256);
// BK Ok
function transfer(address to, uint256 value) public returns (bool);
// BK Ok - Event
event Transfer(address indexed from, address indexed to, uint256 value);
}

// BK Ok
contract BasicToken is ERC20Basic {
// BK Ok
using SafeMath for uint256;

// BK Ok
mapping(address => uint256) balances;
address public pendingOwner;

/**
* @dev transfer token for a specified address
* @param _to The address to transfer to.
* @param _value The amount to be transferred.
*/
* @dev Modifier throws if called by any account other than the pendingOwner.
*/
// BK Ok
function transfer(address _to, uint256 _value) public returns (bool) {
// BK Ok
require(_to != address(0));

// SafeMath.sub will throw if there is not enough balance.
modifier onlyPendingOwner() {
// BK Ok
balances[msg.sender] = balances[msg.sender].sub(_value);
require(msg.sender == pendingOwner);
// BK Ok
balances[_to] = balances[_to].add(_value);
// BK Ok - Log event
Transfer(msg.sender, _to, _value);
_;
}

/**
* @dev Allows the current owner to set the pendingOwner address.
* @param newOwner The address to transfer ownership to.
*/
// BK Ok - Only owner can execute
function transferOwnership(address newOwner) onlyOwner public {
// BK Ok
return true;
pendingOwner = newOwner;
}

/**
* @dev Gets the balance of the specified address.
* @param _owner The address to query the the balance of.
* @return An uint256 representing the amount owned by the passed address.
*/
// BK Ok - Constant function
function balanceOf(address _owner) public constant returns (uint256 balance) {
* @dev Allows the pendingOwner address to finalize the transfer.
*/
// BK Ok - Only pending owner can execute
function claimOwnership() onlyPendingOwner public {
// BK Ok - Log event
OwnershipTransferred(owner, pendingOwner);
// BK Ok
owner = pendingOwner;
// BK Ok
return balances[_owner];
pendingOwner = 0x0;
}
}

// BK Ok
contract ERC20Basic {
// BK Ok
uint256 public totalSupply;
// BK Ok
function balanceOf(address who) public constant returns (uint256);
// BK Ok
function transfer(address to, uint256 value) public returns (bool);
// BK Ok - Event
event Transfer(address indexed from, address indexed to, uint256 value);
}

// BK Ok
contract PresaleOracles is Ownable {
contract PresaleOracles is Claimable {
/*
* PresaleOracles
* Simple Presale contract
Expand All @@ -167,7 +165,6 @@ contract PresaleOracles is Ownable {
uint256 public startTime;
uint256 public endTime;
uint256 public cap;
uint256 public rate;
uint256 public totalInvestedInWei;
uint256 public minimumContribution;
// BK Next 2 Ok
Expand Down Expand Up @@ -211,6 +208,7 @@ contract PresaleOracles is Ownable {
vault = _vault;
}
//TESTED by Roman Storm
event Contribution(address indexed investor, uint256 investorAmount, uint256 investorTotal, uint256 totalAmount);
// BK Ok - Payable
function buy() public payable {
// BK Ok
Expand All @@ -229,6 +227,8 @@ contract PresaleOracles is Ownable {
totalInvestedInWei += msg.value;
// BK Ok
forwardFunds(msg.value);
// BK Ok
Contribution(msg.sender, msg.value, investorBalances[investor], totalInvestedInWei);
}

//TESTED by Roman Storm
Expand All @@ -249,7 +249,7 @@ contract PresaleOracles is Ownable {
}

// BK Ok
BasicToken token = BasicToken(_token);
ERC20Basic token = ERC20Basic(_token);
// BK Ok
uint256 balance = token.balanceOf(this);
// BK Ok
Expand Down Expand Up @@ -317,5 +317,4 @@ contract PresaleOracles is Ownable {
}
}


```
15 changes: 9 additions & 6 deletions audit/test/01_test1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -172,16 +172,17 @@ console.log("RESULT: ");
var sendContribution1Message = "Send Contribution";
// -----------------------------------------------------------------------------
console.log("RESULT: " + sendContribution1Message);
var sendContribution1_1Tx = eth.sendTransaction({from: account3, to: saleAddress, gas: 400000, value: web3.toWei("1000", "ether")});
var sendContribution1_1Tx = eth.sendTransaction({from: account3, to: saleAddress, gas: 400000, value: web3.toWei("999", "ether")});
var sendContribution1_2Tx = eth.sendTransaction({from: account4, to: saleAddress, gas: 400000, value: web3.toWei("1000", "ether")});
var sendContribution1_3Tx = eth.sendTransaction({from: account5, to: saleAddress, gas: 400000, value: web3.toWei("1000", "ether")});
var sendContribution1_4Tx = eth.sendTransaction({from: account6, to: saleAddress, gas: 400000, value: web3.toWei("1000", "ether")});
while (txpool.status.pending > 0) {
}
var sendContribution1_5Tx = eth.sendTransaction({from: account7, to: saleAddress, gas: 400000, value: web3.toWei("8000.00000000001", "ether")});
var sendContribution1_5Tx = eth.sendTransaction({from: account3, to: saleAddress, gas: 400000, value: web3.toWei("1", "ether")});
var sendContribution1_6Tx = eth.sendTransaction({from: account7, to: saleAddress, gas: 400000, value: web3.toWei("8001.00000000001", "ether")});
while (txpool.status.pending > 0) {
}
var sendContribution1_6Tx = eth.sendTransaction({from: account7, to: saleAddress, gas: 400000, value: web3.toWei("7999", "ether")});
var sendContribution1_7Tx = eth.sendTransaction({from: account7, to: saleAddress, gas: 400000, value: web3.toWei("7999", "ether")});
while (txpool.status.pending > 0) {
}
printTxData("sendContribution1_1Tx", sendContribution1_1Tx);
Expand All @@ -190,13 +191,15 @@ printTxData("sendContribution1_3Tx", sendContribution1_3Tx);
printTxData("sendContribution1_4Tx", sendContribution1_4Tx);
printTxData("sendContribution1_5Tx", sendContribution1_5Tx);
printTxData("sendContribution1_6Tx", sendContribution1_6Tx);
printTxData("sendContribution1_7Tx", sendContribution1_7Tx);
printBalances();
failIfTxStatusError(sendContribution1_1Tx, sendContribution1Message + " ac3 1000 ETH");
failIfTxStatusError(sendContribution1_1Tx, sendContribution1Message + " ac3 999 ETH");
passIfTxStatusError(sendContribution1_2Tx, sendContribution1Message + " ac4 1000 ETH - Expecting failure, not whitelisted");
failIfTxStatusError(sendContribution1_3Tx, sendContribution1Message + " ac5 1000 ETH");
passIfTxStatusError(sendContribution1_4Tx, sendContribution1Message + " ac6 1000 ETH - Expecting failure, not whitelisted");
passIfTxStatusError(sendContribution1_5Tx, sendContribution1Message + " ac7 8000.00000000001 ETH - Expecting failure, amount will blow the cap");
failIfTxStatusError(sendContribution1_6Tx, sendContribution1Message + " ac7 7999 ETH");
failIfTxStatusError(sendContribution1_5Tx, sendContribution1Message + " ac3 1 ETH");
passIfTxStatusError(sendContribution1_6Tx, sendContribution1Message + " ac7 8000.00000000001 ETH - Expecting failure, amount will blow the cap");
failIfTxStatusError(sendContribution1_7Tx, sendContribution1Message + " ac7 7999 ETH");
printPresaleContractDetails();
console.log("RESULT: ");
Expand Down
Loading

0 comments on commit 862367c

Please sign in to comment.