Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go-base32 encoding is broken with no padding and an encoder to io.Writer #62

Open
MichaelMure opened this issue Oct 8, 2024 · 2 comments
Labels
kind/bug A bug in existing code (including security flaws) need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization

Comments

@MichaelMure
Copy link

For example with b32.NewEncoder(b32.NewEncodingCI("abcdefghijklmnopqrstuvwxyz234567").WithPadding(b32.NoPadding), out), encoding Decentralize everything!!! yield birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbeeswgzlo instead of birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbee.

Note the swgzlo suffix, which is taken from the beginning of the encoding buffer, bytes 3 to 8.

This has been fixed upstream (in the go stdlib) with golang/go@10529a0.

To be honest, I don't see the point why go-base32 exist anymore, as the original reason for the fork was to "add option for raw encoding", which exist in the stdlib since golang/go@5f4f751

Maybe go-base32 should be retired entirely?

@lidel lidel added need/triage Needs initial labeling and prioritization kind/bug A bug in existing code (including security flaws) labels Oct 8, 2024
@lidel
Copy link
Member

lidel commented Oct 8, 2024

Retiring sounds sensible. Are you willing to open a PR that switches to stdlib?
Or could it be a part of #44?

We could then release and archive https://github.com/multiformats/go-base32

@lidel lidel added the need/author-input Needs input from the original author label Oct 8, 2024
@MichaelMure
Copy link
Author

@lidel FYI I tried, but the problem is that go-base32 has case-insensitive decoding, meaning that for example birswgzloorzgc3djpjssazlwmvzhs5dinfxgoijbeeswGZLO could be decoded, even if strictly speaking it's outside of the alphabet you expect for the multibase.
I don't really see why it's important, but it seems like there has been some effort to have it that way.

The funny thing is that there is an argument to have it in encoding/base32: encoding/hex, which also has a case insensitive alphabet does support case insensitive decoding. Why not base32 then? It's a bit more than I'm willing to do on my detour or my detour here though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) need/author-input Needs input from the original author need/triage Needs initial labeling and prioritization
Projects
None yet
Development

No branches or pull requests

2 participants