OriginProtocol / dshop

Origin Dshop - launch your own decentralized store
https://www.originprotocol.com/dshop
MIT License
138 stars 87 forks source link

Corrupt shop configs #358

Closed nick closed 4 years ago

nick commented 4 years ago

Seems like some of the encrypted shop configs are unable to be JSON.parse'ed, possibly due to certain characters like emoji's causing issues during the encryption phase.

Currently corrupt configs:

node scripts/configCli.js --networkId=1 -a --operation=checkConfig

Dump config as a string:

node scripts/configCli.js --networkId=1 --shopId=ID --operation=getRaw
mikeshultz commented 4 years ago

UTF-8 emojis seem fine? Assuming I didn't chunk up the characters here:

> JSON.parse(JSON.stringify('Shop name is ♥️'))
'Shop name is ♥️'
> JSON.parse(JSON.stringify('🤟 rawk'))
'🤟 rawk'

I loaded the string up in Python so I could "see" the character in a bytestring(transferred after being base64 encoded because copy & paste was removing the offending character) and saw this:

https://test-shop-one-e\x0f.ogn.app/test-shop-one-e\x0f/

This appears to be a special "shift in" control character inserted into the shop slug. Seems to be a way to switch between character sets, like for emojis:

Shift In is also used in the 2G variant[4] of SoftBank Mobile's encoding for emoji. 

Looking at that slug it looks like we made an attempt to normalize the string, perhaps some characters were removed/replaced but we missed control characters?

nick commented 4 years ago

Here's a test case. Note I had to add getIV to the exports in encryptedConfig

const encConfig = require('./utils/encryptedConfig')

const iv = encConfig.getIV()

const original = { abc: '234-😬' }
const stringified = JSON.stringify(original)
const encrypted = encConfig.encrypt(iv, stringified)
const decrypted = encConfig.decrypt(iv, encrypted)
const decryptedParsed = JSON.parse(decrypted)

console.log({ original, stringified, encrypted, decrypted, decryptedParsed })

Output

{ original: { abc: '234-😬' },
  stringified: '{"abc":"234-😬"}',
  encrypted:
   'adbd737b242f2693246c85058b0c2c5c27ad2029e9b5187bf2a6788625591d70',
  decrypted: '{"abc":"234-=,"}',
  decryptedParsed: { abc: '234-=,' } }
nick commented 4 years ago

360 seems to fix the test case, though there might be a better way to handle this. I guess we'd need to update everything encrypted in the DB if we make this change or something similar.

mikeshultz commented 4 years ago

Looks like #360 works because instead of binary encoding you're using utf8. I think we can make this change without breaking any shops that are purely ASCII. Any utf-8 will bugger it, however.

EDIT: To be clear, the change I'm talking about is changing the "original" encoding to utf8 but leaving the encrypted binary encoding as hex.

mikeshultz commented 4 years ago

Also interesting, there's other non-ascii shop names that don't cause this issue. I guess it's only these control characters?

SELECT id, name FROM shops WHERE name ~ '[^[:ascii:]]';

None of these shops have active deployments though. I'd be comfortable making this change from binary to utf8. Thoughts?

nick commented 4 years ago

Sounds good to me