leather-io / extension

Leather browser extension
https://leather.io
MIT License
293 stars 140 forks source link

ZWJ sequence emoji's: Warn when buying or flag address "fake" when sending #2962

Open 314159265359879 opened 1 year ago

314159265359879 commented 1 year ago

User is requesting a feature on the Hiro wallet such as the one below to avoid scams with "fake domains"

A. Warning when a user tries to buy unicode domain, that is a 'copy' of another emoij due to ZWJ Sequence. Alien: xn--cr8h.stx https://gamma.io/collections/bns/xn--cr8h.stx

Alien with other skin tone: xn--cr8hv5640c.stx https://gamma.io/collections/bns/xn--cr8hv5640c.stx

B. A warning when sending to such an address.

Opensea and Metamask do this: image

image

Preliminary analysis: A. I am not sure where we would put such a warning on the wallet, on the sign screen where we show "allow mode warning"?. Perhaps it would be better to add something like that to a marketplace, as the example above (from opensea?). Is this still an issue if the support was for all emoji's including the skin toned ones?

B. Example if you add Alien emoji (👽.stx) on the send address field in Hiro Wallet it doesn't currently translate to a public address. I think that is a good protection against sending to the wrong address already, right?

See difference in address field use of emoji: (the mapped Stacks address is not shown, if you send "preview/confirm" an error is shown) image

And use of unicode: (the mapped Stacks address is shown) image

Best practise would be to use the BNS name to send, instead of the emoji? When we add further support for unicode emoji's we should consider the consequences here.

314159265359879 commented 1 year ago

Do I understand correctly that this is different with Metamask? Using 👽.eth the address field in the Metamask wallet: it would translate to xn--cr8h.eth so a user may (get scammed) buying the xn--cr8hv5640c.eth domain thinking it is 👽.eth and then later send or request funds to it going to the xn--cr8h.eth owner?

If we start warning for this, I think there are a lot more things that could be added in the future. For example this one (that includes persian and arabic numbers): image

That looks a lot like (original, all arabic numbers) image

markmhendrickson commented 1 year ago

This seems like a possible vulnerability to resolve as part of https://github.com/hirosystems/stacks-wallet-web/issues/2872 rather than with the current feature set, since as you mention, users can't enter emojis etc. into the send field currently in any case.

314159265359879 commented 1 year ago

Apparently I didn't have all the information here. I think this is the issue we should solve right away: image

This is an account with seemingly four exactly the same emoji's. Non existent ZWJ emoji's the problem is that the alien emotion doesn't exist with different skin tones. Same with the one shown in the screenshot above.

A quotes from dear Nat: image

image

Do we need help and a script from Adraffy mentioned by Nat or will this suffice to resolve the issue in our wallet? @markmhx

I believe in this case it should only show an emoji in for the original domain and only show ZWJ if they are actually specified for a punycode (cited list in the first post emojipedia).

314159265359879 commented 1 year ago

The wallet team cannot take on building a custom script to support emoji's. They are considered a fun addition for wallet users and the wallet team has other priorities.

That said we are open to integrating an alternative NPM package if someone can make a clear case that it conforms more correctly to the punycode standard.

We are currently using this NPM package for the translation of punycode to emoji's:

This is the lib, https://www.npmjs.com/package/punycode and using it's toUnicode method to do the conversion in this file, https://github.com/hirosystems/stacks-wallet-web/blob/dev/src/app/common/hooks/account/use-account-names.ts

nattaja commented 1 year ago

hi everyone, thank for topic, this is the problem what need to be solved ASAP. My proposal - Gamma team should add Single Punymoji Club to the Club category as they did with other Clubs. This will separate fake domains, as they have more then one punycode length and will separate other namespaces from .btc domains, so people will not be confused as it's continuously happen now.

314159265359879 commented 1 year ago

Adding this here for discussion.

image

nattaja commented 1 year ago

yes this must be clarified by Mechanism team

markmhendrickson commented 1 year ago

cc @hstove @jeffdomke re: BNSx above

314159265359879 commented 1 year ago

Update from the BNS DAO discord: image

dytsui commented 1 year ago

Suppose hiro wallet input emoji domain Same as sendstx.com. If wallet does not use the warning tag. an example: visa.btc. Input and copy paste are different wallet addresses. How do you guys prevent this? 741255F2-595C-4883-92B7-614EEBCD8E41 I think the wallet don’t has a warning :warning:. is not feasible

dytsui commented 1 year ago

I think including zwj should give ⚠️tag(Market/Wallet/Chain).The real zwj emoji in the library should not give ⚠️tag.

314159265359879 commented 1 year ago

Suggestion from the BNSDao discord: image

Seems like a reasonable enhancement to help users check their punycodes maps to the emoji they think.

dytsui commented 1 year ago

Converter problem. Real zwj emoji https://emojipedia.org/emoji-zwj-sequence/. Mint has been completed. All follow gamma/wallet/sendstx and project team devs' suggestion mint's .so switch converter? What should real zwj emoji holders do? 262FDF75-9153-4E11-A68F-8845D5448DFA 4969A9D5-7B33-4857-93DC-6B6E77FEA77D

web3dev3id commented 1 year ago

It's worth noting that there are the emoji zwj sequences you link to above which alter the appearance of emojis (combining two or changing skin tone), but there are also a handful of specific emojis that make use of zwj without being classed as a sequence. The only ones I know of are the single digit set approved in Unicode 1.1 and added to Emoji 2.0 in 2015.

Example here: https://emojipedia.org/digit-one/

nattaja commented 1 year ago

It's worth noting that there are the emoji zwj sequences you link to above which alter the appearance of emojis (combining two or changing skin tone), but there are also a handful of specific emojis that make use of zwj without being classed as a sequence. The only ones I know of are the single digit set approved in Unicode 1.1 and added to Emoji 2.0 in 2015.

Example here: https://emojipedia.org/digit-one/

that's why they don't want to use unicode visualisation as wallet address, instead will be used punycode as wallet address, but they can resolve it to unicode after you post punycode to double check receiver address

nattaja commented 1 year ago

It's worth noting that there are the emoji zwj sequences you link to above which alter the appearance of emojis (combining two or changing skin tone), but there are also a handful of specific emojis that make use of zwj without being classed as a sequence. The only ones I know of are the single digit set approved in Unicode 1.1 and added to Emoji 2.0 in 2015.

Example here: https://emojipedia.org/digit-one/

punycode will be added back to wallets

dytsui commented 1 year ago

Suggestion from the BNSDao discord: image

Seems like a reasonable enhancement to help users check their punycodes maps to the emoji they think.

Nat speaks very intelligently. can temporarily accept this idea. It can be used on the testnet as soon as possible. But it will not accept changing the converter. You display your mint. don’t need to change anything for your bug mint

nattaja commented 1 year ago

Suggestion from the BNSDao discord: image

Seems like a reasonable enhancement to help users check their punycodes maps to the emoji they think.

bns hiro emoji address valid

yes, please! right now difficult to understand what emoji used as wallet address when adding the code only!

dytsui commented 1 year ago

It looks good! But fake mint they also show the same emoji.Will make more people go mint fake.a long-term plan, it is best to have a ⚠️tag for zwj. The official list of real zwj emojis does not require a warning tag

dytsui commented 1 year ago

Even with punycode code use pay method. ⚠️tag is required. Even without the ⚠️tag.fake emoji should not appear in the input analyze address bar.Let fake mint know: fake is forever fake

dytsui commented 1 year ago

If you guys disagree has a ⚠️tag.fake should be forever this.never parse fake 02469FAF-CC80-42F9-BB52-11D0FFCAB521

314159265359879 commented 1 year ago

I received this today from a valued member of the community. image all this digits are fake made with fe0f this is not my wallet, people who minted posted in the chat could you not show visualisation for fake digits at least? we will have scammed buyers and a lot of FUD https://adraffy.github.io/punycode.js/test/demo.html#u=1%EF%B8%8F code content digit+fe0f, nothing else

I have asked the team how they would feel about not rendering some of these codes such as a "fake" 21.btc.

  1. Doing something about this is not going to be a priority anytime soon. https://github.com/hirosystems/wallet/issues/2962#issuecomment-1401658513
  2. Although we can consider adding labeling punycode values that render in a confusing way. The risk profile is very different in the wallet vs. somewhere public like Gamma. The wallet UI shows punycode->unicode conversions for a particular user who already bought / obtained these relevant BNS names. It does not show them for people who haven't yet bought them but might be deceived into doing so. So, we could enhance this display, there's theoretically little to no surface area here for things to go wrong at the moment, as far as we can tell.
  3. Although it may be frustrating, this is how punycode works. It is a legit use of the punycode protocol. Should we really be blocking part of that protocol because some users do not like the way it works?

As I have mentioned before our team can't prioritize work on this. Not even the suggestions above that could improve the user experience such as https://github.com/hirosystems/wallet/issues/2962#issuecomment-1467801838 but we are open to reviewing PR's send by the community to implement such an improvement.

dytsui commented 1 year ago

if do not agree with the tag'fake'❌⚠️. Can you mark the real one as 'true'✅?