From 823820530ad2f98bcaaa56edbfd4ca4559d5e2e2 Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 8 Mar 2018 19:05:06 -0800 Subject: [PATCH 1/2] Make SetToken constructor test pass. --- test/setToken.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/setToken.js b/test/setToken.js index c5a2b786a..8fd3a28a5 100644 --- a/test/setToken.js +++ b/test/setToken.js @@ -41,7 +41,7 @@ contract('{Set}', function(accounts) { it('should not allow creation of a {Set} with no inputs', async () => { return expectedExceptionPromise( - () => SetToken.new([], [], { from: testAccount }), + () => SetToken.new([], [], name, symbol, { from: testAccount }), 3000000, ); }); From d8488c0172a01162f0eea289fd53d27c41deb76a Mon Sep 17 00:00:00 2001 From: Jim Posen Date: Thu, 8 Mar 2018 18:57:23 -0800 Subject: [PATCH 2/2] Use SafeMath to prevent multiplication overflow. --- contracts/SetToken.sol | 6 ++++-- test/setToken.js | 36 +++++++++++++++++++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/contracts/SetToken.sol b/contracts/SetToken.sol index e0a23ecf6..19a90b350 100644 --- a/contracts/SetToken.sol +++ b/contracts/SetToken.sol @@ -71,7 +71,8 @@ contract SetToken is StandardToken, DetailedERC20("", "", 18), Set { uint currentUnits = units[i]; // The transaction will fail if any of the tokens fail to transfer - assert(ERC20(currentToken).transferFrom(msg.sender, this, currentUnits * quantity)); + uint transferValue = SafeMath.mul(currentUnits, quantity); + assert(ERC20(currentToken).transferFrom(msg.sender, this, transferValue)); } // If successful, increment the balance of the user’s {Set} token @@ -107,7 +108,8 @@ contract SetToken is StandardToken, DetailedERC20("", "", 18), Set { uint currentUnits = units[i]; // The transaction will fail if any of the tokens fail to transfer - assert(ERC20(currentToken).transfer(msg.sender, currentUnits * quantity)); + uint transferValue = SafeMath.mul(currentUnits, quantity); + assert(ERC20(currentToken).transfer(msg.sender, transferValue)); } LogRedemption(msg.sender, quantity); diff --git a/test/setToken.js b/test/setToken.js index 8fd3a28a5..4d454ec79 100644 --- a/test/setToken.js +++ b/test/setToken.js @@ -3,6 +3,8 @@ const assert = require('chai').assert; const SetToken = artifacts.require('SetToken'); const StandardTokenMock = artifacts.require('StandardTokenMock'); +const BigNumber = require('bignumber.js'); + const expectedExceptionPromise = require('./helpers/expectedException.js'); web3.eth.getTransactionReceiptMined = require('./helpers/getTransactionReceiptMined.js'); @@ -141,7 +143,6 @@ contract('{Set}', function(accounts) { assert.exists(setToken, 'Set Token does not exist'); }); - it('should have the basic information correct', async () => { // Assert correct name let setTokenName = await setToken.name({ from: testAccount }); @@ -280,5 +281,38 @@ contract('{Set}', function(accounts) { assert.strictEqual(postIssueBalanceIndexofOwner.toString(), '0'); }); } + + it('should disallow issuing a quantity of tokens that would trigger an overflow', async () => { + var units = 2; + + // This creates a SetToken with only one backing token. + setToken = await SetToken.new( + [tokenB.address], + [units], + setName, + setSymbol, + { from: testAccount }, + ); + + var quantity = 100; + var quantityB = quantity * units; + + await tokenB.approve(setToken.address, quantityB, { + from: testAccount, + }); + + // Set quantity to 2^254 + 100. This quantity * 2 will overflow a + // uint256 and equal 200. + var overflow = new BigNumber('0x8000000000000000000000000000000000000000000000000000000000000000'); + var quantityOverflow = overflow.plus(quantity); + + await expectedExceptionPromise( + () => + setToken.issue(quantityOverflow, { + from: testAccount, + }), + 3000000, + ); + }); }); });