Skip to content
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

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

Closed
ethLouise opened this issue Mar 1, 2017 · 5 comments
Closed

Comments

@ethLouise
Copy link

ethLouise commented Mar 1, 2017

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

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

karalabe commented Mar 2, 2017

I already pointed you to the commit that changes the behavior and the reason behind it: b59c839

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.

@karalabe karalabe closed this as completed Mar 2, 2017
@gatb27
Copy link

gatb27 commented Apr 3, 2017

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

10say commented Apr 13, 2017

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

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: #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.

tyleryasaka pushed a commit to tyleryasaka/identity-playground that referenced this issue Apr 9, 2018
This makes claims valid only for the intended recipient and claim type.

The prefix stuff here is being done to accomodate the prefixing done by
web3 when signing messages, as a security measure
(ethereum/go-ethereum#3731). Because this
signing is most likely going to be done on servers of major claim
validators rather than from end-user clients, the web3 prefix could
probably be bypassed. But to keep things simple I just adjusted the
solidity code to use this prefix. I think it's adequate for now.
nick pushed a commit to OriginProtocol/origin-playground that referenced this issue Apr 9, 2018
This makes claims valid only for the intended recipient and claim type.

The prefix stuff here is being done to accomodate the prefixing done by
web3 when signing messages, as a security measure
(ethereum/go-ethereum#3731). Because this
signing is most likely going to be done on servers of major claim
validators rather than from end-user clients, the web3 prefix could
probably be bypassed. But to keep things simple I just adjusted the
solidity code to use this prefix. I think it's adequate for now.
@CryptoKiddies
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants
@karalabe @EvilJordan @10say @gatb27 @ethLouise @CryptoKiddies and others