Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved bounty tests. #1350

Merged
merged 3 commits into from
Sep 26, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,24 @@ pragma solidity ^0.4.24;
// solium-disable-next-line max-len
import {BreakInvariantBounty, Target} from "../bounties/BreakInvariantBounty.sol";

contract SecureInvariantTargetMock is Target {
function checkInvariant() public returns(bool) {
contract TargetMock is Target {
bool private pwned;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, change to exploited or something less 1337.


function exploitVulnerability() public {
pwned = true;
}

function checkInvariant() public returns (bool) {
if (pwned) {
return false;
}

return true;
}
}

contract SecureInvariantTargetBounty is BreakInvariantBounty {
contract BreakInvariantBountyMock is BreakInvariantBounty {
function _deployContract() internal returns (address) {
return new SecureInvariantTargetMock();
return new TargetMock();
}
}
18 changes: 0 additions & 18 deletions contracts/mocks/InsecureInvariantTargetBounty.sol

This file was deleted.

141 changes: 77 additions & 64 deletions test/BreakInvariantBounty.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ const { ethGetBalance, ethSendTransaction } = require('./helpers/web3');
const expectEvent = require('./helpers/expectEvent');
const { assertRevert } = require('./helpers/assertRevert');

const SecureInvariantTargetBounty = artifacts.require('SecureInvariantTargetBounty');
const InsecureInvariantTargetBounty = artifacts.require('InsecureInvariantTargetBounty');
const BreakInvariantBountyMock = artifacts.require('BreakInvariantBountyMock');
const TargetMock = artifacts.require('TargetMock');

require('chai')
.use(require('chai-bignumber')(web3.BigNumber))
Expand All @@ -17,82 +17,95 @@ const sendReward = async (from, to, value) => ethSendTransaction({

const reward = new web3.BigNumber(web3.toWei(1, 'ether'));

contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) {
context('against secure contract', function () {
beforeEach(async function () {
this.bounty = await SecureInvariantTargetBounty.new({ from: owner });
});

it('can set reward', async function () {
await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.equal(reward);
});

context('with reward', function () {
beforeEach(async function () {
const result = await this.bounty.createTarget({ from: researcher });
const event = expectEvent.inLogs(result.logs, 'TargetCreated');

this.targetAddress = event.args.createdAddress;

await sendReward(owner, this.bounty.address, reward);

const balance = await ethGetBalance(this.bounty.address);
balance.should.be.bignumber.equal(reward);
});
contract('BreakInvariantBounty', function ([_, owner, researcher, anyone, nonTarget]) {
beforeEach(async function () {
this.bounty = await BreakInvariantBountyMock.new({ from: owner });
});

it('cannot claim reward', async function () {
await assertRevert(
this.bounty.claim(this.targetAddress, { from: researcher }),
);
});
});
it('can set reward', async function () {
await sendReward(owner, this.bounty.address, reward);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this sendReward name. What would you think of dropping the function and just calling web3.eth.sendTransaction?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened #1358

(await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(reward);
});

context('against broken contract', function () {
context('with reward', function () {
beforeEach(async function () {
this.bounty = await InsecureInvariantTargetBounty.new();

const result = await this.bounty.createTarget({ from: researcher });
const event = expectEvent.inLogs(result.logs, 'TargetCreated');

this.targetAddress = event.args.createdAddress;
await sendReward(owner, this.bounty.address, reward);
});

it('can claim reward', async function () {
await this.bounty.claim(this.targetAddress, { from: researcher });
const claim = await this.bounty.claimed();

claim.should.equal(true);

const researcherPrevBalance = await ethGetBalance(researcher);
describe('destroy', function () {
it('returns all balance to the owner', async function () {
const ownerPreBalance = await ethGetBalance(owner);
await this.bounty.destroy({ from: owner, gasPrice: 0 });
const ownerPostBalance = await ethGetBalance(owner);

await this.bounty.withdrawPayments(researcher, { gasPrice: 0 });
const updatedBalance = await ethGetBalance(this.bounty.address);
updatedBalance.should.be.bignumber.equal(0);
(await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0);
ownerPostBalance.sub(ownerPreBalance).should.be.bignumber.equal(reward);
});

const researcherCurrBalance = await ethGetBalance(researcher);
researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward);
it('reverts when called by anyone', async function () {
await assertRevert(this.bounty.destroy({ from: anyone }));
frangio marked this conversation as resolved.
Show resolved Hide resolved
});
});

it('cannot claim reward from non-target', async function () {
await assertRevert(
this.bounty.claim(nonTarget, { from: researcher })
);
});
describe('claim', function () {
it('is initially unclaimed', async function () {
(await this.bounty.claimed()).should.equal(false);
});

it('can create claimable target', async function () {
const { logs } = await this.bounty.createTarget({ from: researcher });
expectEvent.inLogs(logs, 'TargetCreated');
frangio marked this conversation as resolved.
Show resolved Hide resolved
});

context('reward claimed', function () {
beforeEach(async function () {
await this.bounty.claim(this.targetAddress, { from: researcher });
context('with target', async function () {
beforeEach(async function () {
const { logs } = await this.bounty.createTarget({ from: researcher });
const event = expectEvent.inLogs(logs, 'TargetCreated');
this.target = TargetMock.at(event.args.createdAddress);
});

context('before exploiting vulnerability', async function () {
it('reverts when claiming reward', async function () {
await assertRevert(this.bounty.claim(this.target.address, { from: researcher }));
});
});

context('after exploiting vulnerability', async function () {
beforeEach(async function () {
await this.target.exploitVulnerability({ from: researcher });
});

it('sends the reward to the researcher', async function () {
await this.bounty.claim(this.target.address, { from: anyone });

const researcherPreBalance = await ethGetBalance(researcher);
await this.bounty.withdrawPayments(researcher);
const researcherPostBalance = await ethGetBalance(researcher);
frangio marked this conversation as resolved.
Show resolved Hide resolved

researcherPostBalance.sub(researcherPreBalance).should.be.bignumber.equal(reward);
(await ethGetBalance(this.bounty.address)).should.be.bignumber.equal(0);
});

context('after claiming', async function () {
beforeEach(async function () {
await this.bounty.claim(this.target.address, { from: researcher });
});

it('is claimed', async function () {
(await this.bounty.claimed()).should.equal(true);
});

it('no longer accepts rewards', async function () {
await assertRevert(sendReward(owner, this.bounty.address, reward));
});
frangio marked this conversation as resolved.
Show resolved Hide resolved
});
});
});

it('should no longer be payable', async function () {
await assertRevert(
sendReward(owner, this.bounty.address, reward)
);
context('with non-target', function () {
it('reverts when claiming reward', async function () {
await assertRevert(this.bounty.claim(nonTarget, { from: researcher }));
});
});
});
});
Expand Down