ethereum / go-ethereum

Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
47.49k stars 20.1k forks source link

Signing in geth and verifying in solidity do not produce correct results #3731

Closed ethLouise closed 7 years ago

ethLouise commented 7 years ago

Geth version: 1.5.9-stable OS & Version: Linux Commit hash : a07539fb88db7231d18db918ed7a6a4e32f97450

I sign with personal.sign or web3.eth.sign in geth then I test with personal.ecrecover it works fine, it returns the right address, BUT when I try to verify with solidity it return a wrong address !! can you tell me why please!

karalabe commented 7 years ago

I already pointed you to the commit that changes the behavior and the reason behind it: https://github.com/ethereum/go-ethereum/commit/b59c8399fbe42390a3d41e945d03b1f21c1a9b8d

TL;DR; Geth prepends the string \x19Ethereum Signed Message:\n<length of message> to all data before signing it (https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sign). If you want to verify such a signature from Solidity, you'll have to prepend the same string in solidity before doing the ecrecovery.

The reason was that by allowing signing arbitrary data, a DApp can trick the user to sign a transaction masqueraded as some arbitrary application data. This prefix ensures that no matter what data a DApp sends Geth, it cannot be abused as a transaction. To use this mechanism for signature verification in solidity, you'll need to have your contract also do this prefixing.

Alternatively, you might want to do account management client side in your application and not rely on Geth for that. The go-ethereum libraries provide the methods to sign arbitrary data, they are just not exposed by Geth due to security reasons.

gatb27 commented 7 years ago

Hi there, do you have any example on how to pre-append the string on the Solidity side? What I have to sign is always a keccak-256 hash, so the size is always the same :)

10say commented 7 years ago

That worked for me :

bytes32 msgHash = 0x852daa74cc3c31fe64542bb9b8764cfb91cc30f9acf9389071ffb44a9eefde46;
bytes32 r = 0xb814eaab5953337fed2cf504a5b887cddd65a54b7429d7b191ff1331ca0726b1;
bytes32 s = 0x264de2660d307112075c15f08ba9c25c9a0cc6f8119aff3e7efb0a942773abb0;
uint8 v = 0x1b;

bytes memory prefix = "\x19Ethereum Signed Message:\n32";
bytes32 prefixedHash = sha3(prefix, msgHash);

address PK = ecrecover(prefixedHash, v, r, s);
// PK should equal 0xa6fb229e9b0a4e4ef52ea6991adcfc59207c7711
EvilJordan commented 6 years ago

So, is this ever going to be fixed to be compatible with TREZOR and the like? Specifically, using an ASCII message length vs. a var_int/hex length of the message: https://github.com/ethereum/go-ethereum/issues/14794

It seems no matter how I attempt to pass in a prefixed message signed from a TREZOR (using ascii length or a hex length), geth can't verify it.

CryptoKiddies commented 5 years ago

Something that's been bothering me is the potential for abuse with the SignTransaction method. Though it requires to be fed specific named properties in order to execute, it can still be abused by a dapp that bypasses Metamask and uses its own custom wallet handling. It can still display disinformation and provide differing values under the hood, thus having a transaction with arbitrary data signed and ready to using whatever private key the user had sitting in local storage. How does this change protect against the form of abuse I highlight here?