bitshares / bitshares-ui

Fully featured Graphical User Interface / Reference Wallet for the BitShares Blockchain
https://wallet.bitshares.org
MIT License
518 stars 570 forks source link

[1] Security image missing on [Send] screen #801

Closed testlump closed 5 years ago

testlump commented 6 years ago

Description

The security image that was present on the [Send] screen is no longer present in build 171205. This removes a security layer that aimed to prevent users sending to phishing or scam accounts that were similarly named to trap the unwary. E.g. p0loniex-wallet instead of poloniex-wallet.

old wallet send screen image

new wallet send screen image

Steps to reproduce

1) Install build 171205 2) Navigate to Send (Beta) screen 3) No security image is displayed when inputting the destination account name

Expected functionality

When inputting a target account name into the To box, only the target account holders name and membership level is displayed (E.g. BASIC MEMBER {SOMENUM})

On a separate note; The security image is now only visible adjacent to the account name when clicking the account name link in the top right corner. This image should be prominently displayed throughout the UI as it aids users in identifying their account, ideally persistently adjacent to the account name in the top right and adjacent to any fields on any screens where a user enters account names (E.g. Send, Receive, Withdraw in either From or Recipient (To) fields).

Clicking a security image should detail it's purpose - E.g. on a Send screen in the To field, clicking the image adjacent to the field name might say: "This is a security image that helps you identify that you are sending to the correct account. If you are in doubt that you are sending to the correct account, ask the recipient to send you a picture of their security image and check that they match".

wmbutler commented 6 years ago

Are you telling me that people actually send security images to each other and compare them? Honestly, when is the last time anyone did that? Having trusted users in Contacts is really the way to proceed forward on this IMO.

I have no idea what the security image for a recipient is. If we look at the design pattern for all of crypto and banking, there isn't a single other site that uses this.

I'm particularly interested in the fact that you are concerned about the From icon. This is an account where you hold the keys. Are you suggesting that it's possible for a user to SEND from the wrong account because they can't see their icon?

btsfav commented 6 years ago

agree with @wmbutler - and you can still use the old send via menu

testlump commented 6 years ago

Are you telling me that people actually send security images to each other and compare them?

Yes - I can say I certainly do. See moving funds between wallets with accounts located on different devices like a smartphone and desktop.

I have no idea what the security image for a recipient is. If we look at the design pattern for all of crypto and banking, there isn't a single other site that uses this.

You are comparing apples to oranges. Firstly most crypto's do not have the user friendly 'account name' pattern that is used by Bitshares, they use complex alpha-number strings which contain a built in checksum. As a consequence it is very difficult to send to the incorrect target address since a typo will usually result in a checksum mismatch. Secondly; It is mathematically improbable that an attacker will find an address that is sufficiently similar to a recipients intended destination as to be able to trick the user into sending their funds to an attacker controlled address (Ex Bitcoin Private keys: 5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreBF8or94 and 5HpHagT65TZzG1PH3CSu63k8DbpvD8s5ip4nEB3kEsreBKdE2NK yield receiving addresses 1E1NUNmYw1G5c3FKNPd435QmDvuNG3auYk and 1UCZSVufT1PNimutbPdJUiEyCYSiZAD6n respectively). Thirdly; Trusted contacts only works after the fact. You still need to identify the contact you are adding.

There are no such safeguards in place with Bitshares and the security image is a secondary level of protection. As the number of users increases, so will the number of either deliberate or accidental misdirection of funds. This exact problem is faced within the traditional banking system in the UK (see: https://www.theguardian.com/money/2014/apr/24/banks-respond-faster-misdirected-payments) which have faced numerous issues due to user typo's delivering funds to incorrect accounts. Unlike the legacy banking system, there is no way in the crypto world to recover funds.

If we look at the design pattern for all of crypto and banking, there isn't a single other site that uses this.

This is not an excuse for not doing something. Most exchanges are generic when it comes to their treatment of crypto-currency integrations and therefore the likelihood of them implementing functionality above and beyond that which is strictly necessary to facilitate deposits and withdrawals is unlikely. If we want Bitshares to remain usable, it makes sense to ensure that we guard against all reasonable possible user accidents and the security image - as originally envisaged by Cryptonomex - was exactly for that reason.

An example of a wallet that integrates the security images outside of the Bitshares reference client is hte Smartcoins.de mobile wallet from Agorise (formally Bitshares Munich).

I'm particularly interested in the fact that you are concerned about the From icon. This is an account where you hold the keys. Are you suggesting that it's possible for a user to SEND from the wrong account because they can't see their icon?

You're not accounting for wallets that can contain multiple accounts. It is (or historically has been) possible to create a wallet that contains up to 99 named accounts (more with some fixes from abit I think) and the To field can be adjusted to whatever origin account you want to use.

In summary; you should not be considering this functionality from the perspective of an expert in the Bitshares eco-system but from the perspective of the average user - who as a general rule - needs as much protection as possible, mostly from themselves. Misdirected funds will happen and since Bitshares developers already envisaged this as a possibility, removing the security image for what seems purely aesthetic reasons seems to only harm this goal.

wmbutler commented 6 years ago

The identicon is destined for death. Everything one of your concerns is easily solved by:

I do see your argument regarding unique BTC addresses, but I personally disagree that the identicon is more useful than a qr code in conjunction with address book verification.

btsfav commented 6 years ago
testlump commented 6 years ago

I agree that a QR code would solve this problem in a standardized and widely recognized manner, but this isn't present at the moment. If a QR code is going to replace the identicon then I would agree that it would be a preferable solution.

abitmore commented 6 years ago

I agree that we should implement the new feature first before removing the old feature, but not the opposite.

abitmore commented 6 years ago

By the way I don't think QR code is useful when transferring funds on PC. (The reference wallet is mainly a PC wallet.)

Even Ethereum which has QR codes also has an icon for every address and is widely used for HUMAN verification.

startailcoon commented 5 years ago

I think we should enable human verification images again, as it does give an extra level of verification on usernames. Yes, addressbooks and contacts does give support, as @wmbutler mentiones in the thread, though it should still be possible to use security images.

Suggestion is to add an option in settings to turn on/off security images. When security images are on we display them as usual. We have enough space to do this.

Below is a quick trial to add it to the current Send modal.

bild

startailcoon commented 5 years ago

I'd like some feedback on this. I think we should re-introduced these security images since they are part of other sections of the application, like account manager drop-down.

They look good and don't take up any unnecessary space for the user and at the same time they provide some security enhancements for those that do use them.

startailcoon commented 5 years ago

Will submit a PR this milestone to implement these again

startailcoon commented 5 years ago

Account Images re-enabled

Screenshot from 2019-04-23 06-00-16