Right now (2017-07-19, 10pm CEST), there is an alert from Parity that there is an ongoing exploit on their multisig wallet.
What is the cause and mechanism of this exploit?
16
asked Jul 19 '17 at 20:48
Validity Labs - SebastianValidity Labs - Sebastian
6,8582 gold badges26 silver badges58 bronze badges
-
1you could check the diff of the patch comited to fix this vulnerability : github.com/paritytech/parity/commit/… – Badr Bellaj♦ Jul 19 '17 at 21:27
-
github.com/paritytech/parity/commit/… – Badr Bellaj♦ Jul 19 '17 at 22:26
-
Here is a video from Jordan Leigh from Decypher Media with complete reproduction of the attack youtube.com/watch?v=VUH4gRDQYsA – Igor Barinov Jul 25 '17 at 19:12
24
The wallet exposed a function:
// constructor - just pass on the owner array to the multiowned and
// the limit to daylimit
function initWallet(address[] _owners, uint _required, uint _daylimit) {
initDaylimit(_daylimit);
initMultiowned(_owners, _required);
}
which was in charge of initializing the wallet owners. However, the author forgot to include a check to see if the wallet had already been initialized, thus alloing anybody to call this function and completely overwrite the owners[] array, giving themselves full access to the wallet.
answered Jul 19 '17 at 20:57
-
1OMG that is so simple and so bad, thank you so much! – Validity Labs - Sebastian Jul 19 '17 at 21:19
-
2Setting it as internal should have been sufficient and only letting it be called from the constructor would have made this impossible. The solidity devs might need to think about making internal functions internal – Mick de Graaf Jul 19 '17 at 21:51
-
The intended-to-be-external methods are explicitly marked "external", but the intended-to-be-internal methods were unmarked. That suggests that the programmers misunderstood the default -- even though they're the devs of a major wallet. This was probably a bad choice of default in the Solidity language design. – Daira Hopwood Jul 21 '17 at 8:57
