Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

anyone can kill your contract #6995

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

anyone can kill your contract #6995

ghost opened this issue Nov 6, 2017 · 17 comments
Labels
F1-security 🛡 The client fails to follow expected, security-sensitive, behaviour. M8-contracts 🤝 Smart Contracts / Wasm / Solidity. P0-dropeverything 🌋 Everyone should address the issue now.
Milestone

Comments

@ghost
Copy link

ghost commented Nov 6, 2017

I accidentally killed it.

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

@jtakalai
Copy link

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
Copy link
Author

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 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. Z0-unconfirmed 🤔 Issue might be valid, but it’s not yet known. labels Nov 7, 2017
@Office-Julia

This comment has been minimized.

@ghost
Copy link
Author

ghost commented 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 as completed Nov 7, 2017
@ghost ghost reopened this Nov 7, 2017
@hlogeon
Copy link

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
Copy link

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
Copy link

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
Copy link
Author

ghost commented Nov 7, 2017

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

@hlogeon
Copy link

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
Copy link

noxonsu commented Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan

@hlogeon
Copy link

hlogeon commented Nov 7, 2017

"pragma solidity ^0.4.9;" released on 31 Jan

How does it solves problem?

@tomusdrw
Copy link
Collaborator

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 🛡 The client fails to follow expected, security-sensitive, behaviour. M8-contracts 🤝 Smart Contracts / Wasm / Solidity. P0-dropeverything 🌋 Everyone should address the issue now. and removed F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. Z0-unconfirmed 🤔 Issue might be valid, but it’s not yet known. labels Nov 7, 2017
@openethereum openethereum locked and limited conversation to collaborators Nov 7, 2017
@5chdn
Copy link
Contributor

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 as completed Nov 9, 2017
@5chdn 5chdn added this to the 1.9 milestone Nov 13, 2017
@openethereum openethereum unlocked this conversation Nov 13, 2017
@RafaelCosman
Copy link

RafaelCosman commented 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
Copy link

bernardpeh commented Jan 19, 2018

How come the last 2 links no longer work?

@kirushik
Copy link
Collaborator

@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.

@wongwf82
Copy link

wongwf82 commented Jul 22, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F1-security 🛡 The client fails to follow expected, security-sensitive, behaviour. M8-contracts 🤝 Smart Contracts / Wasm / Solidity. P0-dropeverything 🌋 Everyone should address the issue now.
Projects
None yet
Development

No branches or pull requests

11 participants