search

polkadot-js / extension

Simple browser extension for managing Polkadot and Substrate network accounts in a browser. Allows the signing of extrinsics using these accounts. Also provides a simple interface for compliant extensions for dapps.
Apache License 2.0
964 stars 403 forks source link

Enable raw payloads to be signed in extension #1358

Open S0c5 opened 2 months ago

S0c5 commented 2 months ago

this PR closes #1357 by adding a type 'raw' signature that enables developers to sign raw payloads using the 'signRaw' method in the extension and avoid forced data manipulation in the signing process.

S0c5 commented 2 months ago

@Tbaut thanks for such a quick response!!!

What prevents them from letting users sign a TX without showing them they are signing a TX?

I suggest a change in the UI that warns clearly and directly to users that they're about to perform a low-level operation with an unknown consequence, something:

"STOP THERE: You are about to sign an unknown transaction. Make sure you trust the source, as it could potentially drain all your funds."

what do you think?

What prevents you from wrapping your payload with which is easy enough ?

Given I'm crafting a raw substrate transaction with a custom tool called https://github.com/virto-network/virto-sdk/tree/main/sube and using a cross-platform signing library called lib wallet (https://github.com/virto-network/virto-sdk/tree/main/libwallet) which one of the backends we're developing is Polkadot js, we need to sign raw payloads in order to complete the operation and avoid the BadProof error when submiting.

Tbaut commented 2 months ago

The security issue is about having a malicious Dapp. Having a warning on the Dapp doesn't make sense, if it's malicious there'll obviously be no warning.

To sign in to some websites, e.g Polkassembly, they let users sign a non legible random number. This is bad practice and they should let them sign some text containing a random number, but that's what they do. Say Polkassembly is hacked tomorrow and instead of a random number, it's a substrate TX that transfers all the user funds to the attacker. Well users will blind sign, because the extension won't try to decode the TX since it's a raw signing. It is absolutely essential to differenciate raw signing from transaction signing.

Regarding your use case, I don't get why you need raw signing. Why don't you let users sign the TX as a TX?