entropyxyz / entropy-core

Protocol and cryptography development.
https://docs.entropy.xyz/
GNU Affero General Public License v3.0
9 stars 1 forks source link

`/user/receive_key` endpoint can be misused by a malicious validator #652

Closed ameba23 closed 2 weeks ago

ameba23 commented 6 months ago

During registration or proactive refresh a malicious validator could send other members of their subgroup bad data over the /user/receive_key endpoint.

There are a number of nasty things that could happen here:

Write access to mnemonic and secret key

First and foremost, when reading keyshares in sync_kvdb, we check the key against FORBIDDEN_KEYS which include the mnemonic, and x25519 secret. No such check happens in receive_key, which means the validator can delete the mnemonic and secret encryption key of all validators in its subgroup, causing them to be unable to send transactions or decrypt messages.

We can mitigate this by adding a similar forbidden keys check, or storing the mnemonic etc in a separate namespace to keyshares.

Can overwrite legitimate keyshares with garbage

Sending a bad keyshare to another member of the subgroup will likely cause them to get slashed next time they participate in signing message, even though it was not their fault.

Worse still, sending bad keyshares to all other members of the subgroup, causing them to delete their legitimate keyshares, means the malicious validator now holds the only copy of that that share and can hold the user's account to ransom.

This is the hardest one to fix, but there are some basic checks we could do to make this attack much harder. Eg: storing old keyshares until we have successfully signed a message with the new one, and checking the validating key of the keyshare.

Can fill other validator's disks with garbage

There are currently no checks on the size of the payload, and unknown keys will always be accepted. This means the endpoint can repeatedly be called with as much data as fits in an HTTP post request until the validator's disk becomes full.

This could be mitigated by checking whether the given key is a registered or registering account even if it does not currently existing in the database currently, and adding a payload size limit or checking that the payload can be deserialized to a keyshare.

JesseAbram commented 6 months ago

So both these endpoints expect the information to come from the chain and both check against it in validate_endpoint, which hands the validation to the chain and validates that it comes from there, I don't believe this is possible or an issue

https://github.com/entropyxyz/entropy-core/blob/master/crates/threshold-signature-server/src/user/api.rs#L563

HCastano commented 6 months ago

Can overwrite legitimate keyshares with garbage

We can probably address this by implementing https://github.com/entropyxyz/entropy-core/issues/546.

So both these endpoints expect the information to come from the chain and both check against it in validate_endpoint, which hands the validation to the chain and validates that it comes from there, I don't believe this is possible or an issue

I don't really understand how the validate_new_user function helps here. The flow of this attack would be: perform registration as normal, and then in the send_key function send garbage data.

Since receive_key only checks that the info related to registration is signed by the incoming validator then user_registration_info can be filled with whatever (as long as it decodes properly).

HCastano commented 2 weeks ago

This endpoint was removed in https://github.com/entropyxyz/entropy-core/issues/941.