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

Change strMessageMagic to not collide with Bitcoin #70

Open
prusnak opened this issue Apr 1, 2020 · 3 comments
Open

Change strMessageMagic to not collide with Bitcoin #70

prusnak opened this issue Apr 1, 2020 · 3 comments

Comments

@prusnak
Copy link

prusnak commented Apr 1, 2020

Value of strMessageMagic is set to "Bitcoin Signed Message:\n", this does not make sense as it collides with Bitcoin.

See https://github.com/particl/particl-core/blob/master/src/util/validation.cpp#L19

Please change it to something else.

@kewde
Copy link

kewde commented Apr 23, 2020

@prusnak
Mind giving a bit more context? Are there any security concerns that may affect the Trezor security model?

As far as I can tell, if the key derivation happens on the right path, then I presume things should remain secure, but I do understand that it's a bit off to use the same header.

@tecnovert

I'm not exactly sure but changing this header will have implications for the SMSG service right?

@prusnak
Copy link
Author

prusnak commented Apr 24, 2020

As far as I can tell, if the key derivation happens on the right path, then I presume things should remain secure, but I do understand that it's a bit off to use the same header.

This is valid argument for Trezor, but not for the others. For example, does your own code check whether the path used for signing starts with m/44'/44'?

@kewde
Copy link

kewde commented Apr 25, 2020

@prusnak our core application does not store the master seed by default.
We store m/44'/44' rather than m by default.
https://github.com/particl/particl-core/blob/master/src/wallet/rpchdwallet.cpp#L1588

But this may not hold up for other wallets, so I understand the concern.

tecnovert added a commit to tecnovert/particl-core that referenced this issue Nov 22, 2022
tecnovert added a commit to tecnovert/particl-core that referenced this issue Nov 22, 2022
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

2 participants