search

cryptocoinjs / base-x

Encode/decode any base
MIT License
312 stars 75 forks source link

Versions 3.0.5 and above are not backwards compatible #77

Closed siiky closed 3 years ago

siiky commented 3 years ago

A base45 (alphabet: 0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ $%*+-./:) message was failing to decode in version 3.0.8 with the "Non-base45 character" error. It didn't make any sense, because I confirmed the message did not include any non-base45 character, but later concluded the problem was that the encoded message started with a space.

Through git bisect I found 2274e007d4535a7e3f8bfd50560cb0fb0b5e35bb to be the first "bad" commit (introduced in version 3.0.5) -- though I don't understand why just by reading the code. But looking at 3.0.8 the problem is more obvious:

https://github.com/cryptocoinjs/base-x/blob/806ef3f473cb6c9a84559811f411b99cbcf93deb/src/index.js#L69-L70

It's not even "skipping" spaces at the start, it's rejecting them.

The problem seems to be decoding only, not encoding, and only messages that start with a space when encoded (from my very limited experiments). Versions >=3.0.5 correctly encode e.g. a dollar ($) as a space ( ), but then error while decoding.

And this is what I tried:

$ sha512sum -b *random.bin | sort
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.2_decoded_3.0.2_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.2_decoded_3.0.8_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.8_decoded_3.0.2_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.8_decoded_3.0.4_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.8_decoded_3.0.5_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *encoded_3.0.8_decoded_3.0.8_random.bin
72bf47abfcaf5460bfab86b5eee19fbd257dd3078b7d761dc1ff3beac37426e931b1a22b4b98db5bebb7e19a5fe93fc87d050ddf06cbf05ebffe288db756964a *random.bin
d51172558b9d62cb4ac9eb3dfc54fd56776670c80d570626116fb9eb59fa3e315ca961fc32037ad403005de054e363cfa05a9bfd800ccaf808329ab94324c0f3 *encoded_3.0.2_random.bin
d51172558b9d62cb4ac9eb3dfc54fd56776670c80d570626116fb9eb59fa3e315ca961fc32037ad403005de054e363cfa05a9bfd800ccaf808329ab94324c0f3 *encoded_3.0.8_random.bin

random.bin was created with dd if=/dev/urandom of=random.bin bs=1K count=1, and contained spaces in the middle of the encoded message, but not at the start.

junderw commented 3 years ago

I guess the simple way to fix it would be:

  1. When getting info on the alphabet ie LEADER... also get a boolean of hasSpace which would be ALPHABET.indexOf(' ') >= 0
  2. Don't do anything special space-related if hasSpace is true.
  3. Skipping spaces instead of returning early.
  4. Add tests
  5. Push out a patch

Let me know if you're willing to help. I might have time tomorrow.

junderw commented 3 years ago

Please see #78 and approve it.

Also test to make sure the base45 test vectors look sane to you.

Thanks.

siiky commented 3 years ago

Hey, thanks for the changes! Yes, I'll try it out today and report back.