Hu-Fi / Mr.Market

Mr. Market is the exchange oracle of HuFi, and a CeFi crypto bot on Mixin Messenger
https://mr-market-one.vercel.app
GNU Affero General Public License v3.0
1 stars 6 forks source link

Security clientid/securityid #67

Open posix4e opened 4 months ago

BartoszSolkaBD commented 4 months ago

Currently, when executing a strategy, users need to input their clientId and userId. This approach poses a security risk, as a third party could execute strategies for any user if they know the clientId and userId, or even attempt to brute-force them.

This is a significant security concern for the hosted version.

An example of such an attack could manifest as follows: The attacker executes a PureMarketMaking strategy using another user’s funds, manipulating the parameters to ensure profitability for themselves. This action generates a limit order with the user’s funds, allowing the attacker to fulfill the order and reap profits.

Possible solutions:

  1. Lengthening both identifiers would be advisable.
  2. Implementing an “attempts” calculator for a given userId to restrict the number of tries for mismatched userIds and clientIds would be essential. This should be relatively straightforward since regular users would not attempt to alter these identifiers themselves.
  3. Incorporate an authentication layer requiring users to sign a message with their wallet to access the system

We could also consider incorporating a feature to toggle authentication on and off through an environment file. This way, users of the self-hosted version wouldn’t necessarily need to log in.

piotrswierzy commented 3 months ago

@posix4e We should schedule a call to discuss this matter. In our opinion, the most secure approach is to proceed with incorporating an authentication layer. However, it's important to note that this would also alter the Mixin flow, as authentication would be required.

zed-wong commented 3 months ago

@piotrswierzy I think we will need to add an authentication layer, as user connect wallet, we generate a (jwt) token for them, this token can be used for all protected actions of the user. Such as read spot orders by user, cancel spot order by user, and more on the market making and arbitrage side.

We should schedule a call to discuss this matter. In our opinion, the most secure approach is to proceed with incorporating an authentication layer. However, it's important to note that this would also alter the Mixin flow, as authentication would be required.

posix4e commented 3 months ago

Let’s focus on this after April 20th

zed-wong commented 2 months ago

Currently connect wallet within Mr.Market uses PKCE flow for authorization with mixin api. Which means the JWT token is only stored within client side (web app).

We should transit to normal OAuth flow:

  1. user click on connect wallet
  2. web app request mixin api, confirm authorization
  3. redirect to our oauth handler with param code
  4. our oauth handler requests mixin api for JWT token, if correct store token in DB.

With this JWT token we can verify user's identity and use it for auth-required endpoints

example of oauth handler in Go

zed-wong commented 1 month ago