arborwallet / arbor-wallet

Arbor Wallet is a light Chia wallet (SPV) with a focus on user privacy and security.
https://www.arborwallet.com
Apache License 2.0
11 stars 6 forks source link

Does the wallet send the private keys to arborwallet.com? #33

Closed pestotoast closed 2 years ago

pestotoast commented 3 years ago

I have been reviewing the code of this app and I have a few questions:

In wallet-service.dart it looks like the app is sending the private keys to the server:

Future<dynamic> sendXCH(
      {required String privateKey,
      required var amount,
      required String address,required int fee}) async {
    try {
      final responseData = await http.post(
        Uri.parse('${baseURL}/v1/send'),
        headers: <String, String>{
          'Content-Type': 'application/json; charset=UTF-8',
        },
        body: jsonEncode(<String, dynamic>{
          'private_key': privateKey,
          'amount': amount,
          'destination': address,
          'fee': fee
        }),
      );

Is that correct or am I missing something? If yes, that's a huge security risk.

And the recoverWallet function seems to be sending the seed to the server:

// @POST("/v1/recover") and @POST("/v1/wallet") and @POST("v1/blockchain") and @POST("/v1/balance")
  Future<Wallet> recoverWallet(String phrase)async{
    try{

      final recoverKeyResponse = await http.post(
        Uri.parse('${baseURL}/v1/recover'),
        headers: <String, String>{
          'Content-Type': 'application/json; charset=UTF-8',
        },
        body: jsonEncode(<String, String>{
          'phrase': phrase,
        }),
      );

Is that assumption correct? On arborwallet.com you explicitly say that private keys would never leave the device!

tibbon commented 3 years ago

We can't know for sure what the website is actually running, but here's the code that is in theory being run at those two endpoints:

It appears to basically be shelling out the Chia Lisp, as the commands line up with this: https://github.com/Chia-Network/chia-dev-tools/blob/main/README.md#chialisp-commands

But, I don't know this project enough to make correct on what and why this is happening.

To me it doesn't read as malicious, but just an incredibly poor idea. Not only am I unsure of why they are being sent in the first place, but any layer in the middle (CDN, load balancers, proxies, logging, etc) could be intercepting the private key and able to mis-use it.

Again, I'm not ascribing malice here, as I can't make that call - but it seems very risky and error prone. Maybe it was a shortcut to running Chia Lisp commands, but it's not a good shortcut.

scotopic commented 3 years ago

Thanks for taking the time to look at the code and provide feedback.

Once we reimplement all the necessary libraries in Dart we'll have local key/mnemonic generation, local key/mnemonic import/validation, and signing transaction bundles all done locally and remove the equivalent server APIs.

If anyone has technical know-how to implement CLVM in Dart or BLS crypto in Dart ( currently only a C++ lib exists that Chia team has created ) feel free to add a PR/offer to help implement/test that functionality.

scotopic commented 2 years ago

UPDATE: As of Arbor Wallet v2.0.0 we now have everything in-app/locally generating/signing. All backend API is now removed. Please feel free to verify.