/parity-ethereum

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

anyone can kill your contract #6995

Closed
ghost opened this Issue Nov 6, 2017 · 17 comments

Comments

Assignees
No one assigned
Labels
Projects
None yet
Milestone
  1.9 Velocity
10 participants
@jtakalai @Office-Julia @hlogeon @noxonsu @tomusdrw @5chdn @RafaelCosman @bernardpeh @kirushik @wongwf82
@ghost

ghost commented Nov 6, 2017 •
edited by ghost
Edited 1 time
  • @ghost ghost edited Nov 6, 2017 (most recent)

I accidentally killed it.

https://etherscan.io/address/0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4
@jtakalai

This comment has been minimized.

Show comment
Hide comment
@jtakalai

jtakalai Nov 6, 2017

Hmmh, clearly the kill came from registered owner, and required signatures was 0, see initWallet transaction arguments https://etherscan.io/tx/0x05f71e1b2cb4f03e547739db15d080fd30c989eda04d37ce6264c5686e0722c9

jtakalai commented Nov 6, 2017

Hmmh, clearly the kill came from registered owner, and required signatures was 0, see initWallet transaction arguments https://etherscan.io/tx/0x05f71e1b2cb4f03e547739db15d080fd30c989eda04d37ce6264c5686e0722c9
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 6, 2017

Will it effect the dependent multisig wallets? When i query " isowner(<any_addr>)" the multisig wallets returns TRUE.

ghost commented Nov 6, 2017

Will it effect the dependent multisig wallets? When i query " isowner(<any_addr>)" the multisig wallets returns TRUE.

@Office-Julia Office-Julia added F3-annoyance Z0-unconfirmed labels Nov 7, 2017

@Office-Julia

This comment was marked as outdated.

Show comment
Hide comment
@Office-Julia

Office-Julia Nov 7, 2017

Contributor

Hello. May I ask why you decided that anyone can kill the contract?
You're the owner and you can kill the contract as it supposed to be, so it's expected behaviour, isn't it?

Regards,
Julia.
Contributor

Office-Julia commented Nov 7, 2017 •
edited
Edited 1 time
  • @Office-Julia Office-Julia edited Nov 7, 2017 (most recent)

Hello. May I ask why you decided that anyone can kill the contract?
You're the owner and you can kill the contract as it supposed to be, so it's expected behaviour, isn't it?

Regards,
Julia.
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 7, 2017

Hello, first of all i'm not the owner of that contract. I was able to make myself the owner of that contract because its uninitialized.

These (https://pastebin.com/ejakDR1f) multi_sig wallets deployed using Parity were using the library located at "0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4" address. I made myself the owner of "0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4" contract and killed it and now when i query the dependent contracts "isowner(<any_addr>)" they all return TRUE because the delegate call made to a died contract.

I believe some one might exploit.

ghost commented Nov 7, 2017 •
edited by ghost
Edited 2 times
  • @ghost ghost edited Nov 7, 2017 (most recent)
  • @ghost ghost edited Nov 7, 2017

Hello, first of all i'm not the owner of that contract. I was able to make myself the owner of that contract because its uninitialized.

These (https://pastebin.com/ejakDR1f) multi_sig wallets deployed using Parity were using the library located at "0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4" address. I made myself the owner of "0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4" contract and killed it and now when i query the dependent contracts "isowner(<any_addr>)" they all return TRUE because the delegate call made to a died contract.

I believe some one might exploit.

@ghost ghost closed this Nov 7, 2017

@ghost ghost reopened this Nov 7, 2017

@hlogeon

This comment has been minimized.

Show comment
Hide comment
@hlogeon

hlogeon Nov 7, 2017

Hello! We've clashed this problem! Thanks Parity for the great contract again ;)
Any ideas on how can we get our ETH and tokens back from hacked multisig?
I think that we can get ETH back just by killing contract itself but what about tokens?

hlogeon commented Nov 7, 2017

Hello! We've clashed this problem! Thanks Parity for the great contract again ;)
Any ideas on how can we get our ETH and tokens back from hacked multisig?
I think that we can get ETH back just by killing contract itself but what about tokens?
@hlogeon

This comment has been minimized.

Show comment
Hide comment
@hlogeon

hlogeon Nov 7, 2017

For those Parity guys who doesn't believe that this exploit works - check out your library which were used by multiple multisigs: https://etherscan.io/address/0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4#code

hlogeon commented Nov 7, 2017

For those Parity guys who doesn't believe that this exploit works - check out your library which were used by multiple multisigs: https://etherscan.io/address/0x863df6bfa4469f3ead0be8f9f2aae51c91a907b4#code
@hlogeon

This comment has been minimized.

Show comment
Hide comment
@hlogeon

hlogeon Nov 7, 2017

It looks like kill will not work on the contract itself if the library was killed. Nice job, Parity

hlogeon commented Nov 7, 2017

It looks like kill will not work on the contract itself if the library was killed. Nice job, Parity
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 7, 2017

@hlogeon 1. Why kill won't work?
2. Will ether transfer by owners work?

ghost commented Nov 7, 2017

@hlogeon 1. Why kill won't work?
2. Will ether transfer by owners work?
@hlogeon

This comment has been minimized.

Show comment
Hide comment
@hlogeon

hlogeon Nov 7, 2017

@devops199
Because there is onlymanyowners modifier. Which I think refers library. I didin't check why it's not working but the result of calling kill by 3 owners with the same arguments is just nothing.

hlogeon commented Nov 7, 2017

@devops199
Because there is onlymanyowners modifier. Which I think refers library. I didin't check why it's not working but the result of calling kill by 3 owners with the same arguments is just nothing.
@noxonsu

This comment has been minimized.

Show comment
Hide comment
@noxonsu

noxonsu Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan

noxonsu commented Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan
@hlogeon

This comment has been minimized.

Show comment
Hide comment
@hlogeon

hlogeon Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan

How does it solves problem?

hlogeon commented Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan

How does it solves problem?
@tomusdrw

This comment has been minimized.

Show comment
Hide comment
@tomusdrw

tomusdrw Nov 7, 2017

Contributor

Please read the details of the issue here: https://paritytech.io/blog/security-alert.html

We are analysing the situation and will release an update with further details shortly.
Contributor

tomusdrw commented Nov 7, 2017

Please read the details of the issue here: https://paritytech.io/blog/security-alert.html

We are analysing the situation and will release an update with further details shortly.

@tomusdrw tomusdrw added F1-security M8-contracts P0-dropeverything and removed F3-annoyance Z0-unconfirmed labels Nov 7, 2017

@paritytech paritytech locked and limited conversation to collaborators Nov 7, 2017

@KeithSSmith KeithSSmith referenced this issue Nov 7, 2017

Open

Create MultiSig Wallet #7

@5chdn

This comment has been minimized.

Show comment
Hide comment
@5chdn

5chdn Nov 9, 2017

Member

The library is removed from the registry and all current Parity Wallet versions default to the WHG multi-signature wallets.
Member

5chdn commented Nov 9, 2017

The library is removed from the registry and all current Parity Wallet versions default to the WHG multi-signature wallets.

@5chdn 5chdn closed this Nov 9, 2017

@5chdn 5chdn added this to the 1.9 milestone Nov 13, 2017

@paritytech paritytech unlocked this conversation Nov 13, 2017

@RafaelCosman

This comment has been minimized.

Show comment
Hide comment
@RafaelCosman

RafaelCosman Dec 22, 2017

Thought I'd post some resources to help people that come across this thread:

In historical order:

  1. Original Security Alert
  2. Parity Technologies Multi-Sig Wallet Issue Update
  3. A Postmortem on the Parity Multi-Sig Library Self-Destruct
  4. On Classes of Stuck Ether and Potential Solutions

RafaelCosman commented Dec 22, 2017 •
edited by kirushik
Edited 2 times
  • @kirushik kirushik edited Jan 19, 2018 (most recent)
  • @RafaelCosman RafaelCosman edited Dec 22, 2017

Thought I'd post some resources to help people that come across this thread:

In historical order:

  1. Original Security Alert
  2. Parity Technologies Multi-Sig Wallet Issue Update
  3. A Postmortem on the Parity Multi-Sig Library Self-Destruct
  4. On Classes of Stuck Ether and Potential Solutions
@bernardpeh

This comment has been minimized.

Show comment
Hide comment
@bernardpeh

bernardpeh Jan 19, 2018

How come the last 2 links no longer work?

bernardpeh commented Jan 19, 2018 •
edited
Edited 1 time
  • @bernardpeh bernardpeh edited Jan 19, 2018 (most recent)

How come the last 2 links no longer work?
@kirushik

This comment has been minimized.

Show comment
Hide comment
@kirushik

kirushik Jan 19, 2018

Member

@bernardpeh Our bad, blog engine update ruined some of the links. Thanks for reporting.
I took a liberty to fix the links in the comment — it will do as a stopgap measure, but we'll definitely fix the underlying cause as well.
Member

kirushik commented Jan 19, 2018

@bernardpeh Our bad, blog engine update ruined some of the links. Thanks for reporting.
I took a liberty to fix the links in the comment — it will do as a stopgap measure, but we'll definitely fix the underlying cause as well.

@5chdn 5chdn referenced this issue Apr 12, 2018

Merged

Remove kill() function #2

@ksloven ksloven referenced this issue May 8, 2018

Open

Added Suicidal Contract #12

@maraoz maraoz referenced this issue May 15, 2018

Closed

Provide a way to check two contracts share storage layout #63

@ksloven ksloven referenced this issue May 23, 2018

Closed

Add Suicidal Contract #13

@whilei whilei referenced this issue Jun 18, 2018

Open

This might not always be welcome #2

@jschiarizzi jschiarizzi referenced this issue Jul 16, 2018

Merged

Restore Contract Code at 0x863DF6BFa4469f3ead0bE8f9F2AAE51c91A907b4 #999

@wongwf82

This comment has been minimized.

Show comment
Hide comment
@wongwf82

wongwf82 Jul 22, 2018

FYI, the transaction that was submitted here:
https://etherscan.io/tx/0x05f71e1b2cb4f03e547739db15d080fd30c989eda04d37ce6264c5686e0722c9

The transaction Successfully completed here:
https://etherscan.io/tx/0x47f7cff7a5e671884629c93b368cb18f58a993f4b19c2a53a8662e3f1482f690

wongwf82 commented Jul 22, 2018 •
edited
Edited 1 time
  • @wongwf82 wongwf82 edited Jul 22, 2018 (most recent)
  • @wongwf82 wongwf82 created Jul 22, 2018

FYI, the transaction that was submitted here:
https://etherscan.io/tx/0x05f71e1b2cb4f03e547739db15d080fd30c989eda04d37ce6264c5686e0722c9

The transaction Successfully completed here:
https://etherscan.io/tx/0x47f7cff7a5e671884629c93b368cb18f58a993f4b19c2a53a8662e3f1482f690

@rmeissner rmeissner referenced this issue Aug 24, 2018

Open

Secure DelegateCall #1350

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment