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

Message signing discretely malforms provided message #140

Closed
evilfrog opened this issue Oct 4, 2022 · 1 comment
Closed

Message signing discretely malforms provided message #140

evilfrog opened this issue Oct 4, 2022 · 1 comment

Comments

@evilfrog
Copy link

evilfrog commented Oct 4, 2022

Signing message with: https://github.com/Hashpack/hashconnect#sign

What it says it does is it takes dataToSign and signs it.
What it actuall do is (AFAIK) it first JSON.stringify the dataToSign and then signs it.

Extra characters are added discretely to the message (which changes the signature):

const msg = 'Hello World!'; // => Hello World!
const jmsg = JSON.stringify(msg); // => "Hello World!"

The end result is the returned signature cannot be verified (or to be precise: cannot be verified unless you know about JSON.stringify).

It may not be a problem for JS/Node, but it is a problem if you sign a message on fronted with JS and try to verify it on backend (tested with hashgraph/hedera-sdk-go/v2 and crypto/ed25519).

I see two solutions to this problem:

  1. Keep the behavior but document it (with examples of JSON.stringified values for backend devs)
  2. Get rid of the behavior. Remove JSON.stringify and replace string | object with Uint8Array | string where the string is expected to be hex encoded byte array: https://github.com/Hashpack/hashconnect/blob/main/lib/src/message/relayMessage.ts#L115
@jruffer
Copy link

jruffer commented Oct 4, 2022

Had a conversation with Pluto @teacoat on Discord.

try to sign a message which is 'Hello World!', but apparently, the library is JSON.stringifying it, which changes 'Hello World!' into '"Hello World!"' - it adds these double quotes.

We have a work around but just wanted to make sure you were going to use this as it stands?

@teacoat teacoat closed this as completed Jan 10, 2024
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

3 participants