bytesbay / web3-token

Web3 Token is a new way to authenticate users in a hybrid dApps using signed messages. Implementation of EIP-4361.
381 stars 51 forks source link

Tampered body doesn't brake the verification #42

Open herzog0 opened 2 years ago

herzog0 commented 2 years ago

Hi! I was doing some tests to understand what would occur if the body of the message had been tampered and the attacker provided the same signature to the verify function. This snippet illustrates the basics of the attempt:

const token = String(req.header("Authorization"))
const { signature, body } = JSON.parse(Buffer.from(token, "base64").toString())
const tamperedBody = body + "X"

const tamperedToken = Base64.encode(JSON.stringify({
    signature,
    body: tamperedBody,
}))

const { address } = Web3Token.verify(tamperedToken)

console.log("Didn't brake, address is " + address)

And, unfortunately, no exception is thrown. Also, what I find the most confusing part, an actual address is recovered from the payload! Fortunately it is not the same address that signed the message, but still, a valid address.
Am I missing something regarding web3 signatures? Isn't the whole point of signatures to guarantee that a given payload hasn't been tampered?

herzog0 commented 2 years ago

I've downloaded your code and searched a bit, and I've come to the conclusion that tampering can't be detected solely looking at the combo signature+message, the expect public key has to be there too for a true verification, right?
The problem in my case is that I don't have the user's public key prior to their signing up in my platform. I wonder however if a little trick could be done so that it is almost impossible for someone to be tampering the message, which is to generate two tokens instead of one in the browser, each token having a different signature+message pair.
This way, in the backend, I can check that the public addresses recovered from the tokens are the same, therefore those tokens MUST have been generated by the same private key and, therefore, the likelihood of the bodies having been tampered is almost zero.
Do you think this is a good approach?

dvartic commented 1 year ago

I think in the current implementation you should check address against expected address to avoid any potential unwanted behaviour.