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

Jump to bottom

encoding/base32: cannot decode nonpadded encodings #20854

Closed
bobjalex opened this issue Jun 29, 2017 · 10 comments
Closed

encoding/base32: cannot decode nonpadded encodings #20854

bobjalex opened this issue Jun 29, 2017 · 10 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.9Maybe

Comments

@bobjalex
Copy link

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version
go version go1.9beta2 windows/amd64

What operating system and processor architecture are you using (go env)?

go env
set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\GoLib_beta
set GORACE=
set GOROOT=C:\Go_beta
set GOTOOLDIR=C:\Go_beta\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Bob\AppData\Local\Temp\go-build247706827=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config

What did you do?

Tried to decode a base32 nonpadded encoding.

If possible, provide a recipe for reproducing the error.

The attached little program demonstrates the bug:
base32_bug.go.txt

What did you expect to see?

A proper decoding.

What did you see instead?

The error: decoding not padded: illegal base32 data at input byte 8
Even though WithPadding(base32.NoPadding) was specified.
@bradfitz
Copy link
Contributor

That was added in 5f4f751. @zegl, can you take a look?

@bradfitz bradfitz added this to the Go1.9Maybe milestone Jun 29, 2017
@bradfitz bradfitz added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 29, 2017
@bradfitz bradfitz changed the title Base32 cannot decode nonpadded encodings encoding/base32: cannot decode nonpadded encodings Jun 29, 2017
@zegl
Copy link
Contributor

zegl commented Jun 30, 2017 •
edited

@bradfitz: Sure. It was a mistake to not modify Decode to allow non- or custom-padding.

Do you think that the message needs to be padded with the same character as specified by WithPadding(), or should we allow any type of padding (or none at all)? I'd pick the latter option, but it might not be as safe.

Update: In the patch linked below (I'll submit a CL later) the padding for decoding must be set with WithPadding() before attempting to decode. This is also what encoding/base64 does.

zegl pushed a commit to zegl/go that referenced this issue Jun 30, 2017
encoding/base32: support non- and custom-padding in decode()
fb5f92b 
Expects the padding of the input to be the same as set by Encoding.WithPadding()

Fixes golang#20854
@bobjalex
Copy link
Author

bobjalex commented Jun 30, 2017 via email

@zegl
Copy link
Contributor

zegl commented Jun 30, 2017

@bobjalex: The case where the input is a valid base32 encoded message (with = as padding), but no padding is necessary is already covered in the Decode() method shipped in the existing go1.9 betas. (Example: "fooba" => "MZXW6YTB").

In the other case you could look for a character that is not a part of the base32 alphabet, and infer that the first char like that is the one that is the padding char.

zegl added a commit to zegl/go that referenced this issue Jun 30, 2017
@zegl
encoding/base32: support non- and custom-padding in decode()
e61885a 
Expects the padding of the input to be the same as set by Encoding.WithPadding()

Fixes golang#20854
zegl added a commit to zegl/go that referenced this issue Jun 30, 2017
@zegl
encoding/base32: support non- and custom-padding in decode()
f665d79 
Expects the padding of the input to be the same as set by Encoding.WithPadding()

Fixes golang#20854

Signed-off-by: Gustav Westling <gustav@westling.xyz>
@bobjalex
Copy link
Author

A downside to inferring the padding char is that it would somewhat limit the ability to detect corruption. RFC 4648 says

A 33-character subset of US-ASCII is used, enabling 5 bits to be
represented per printable character. (The extra 33rd character, "=",
is used to signify a special processing function.)

That would mean that any character outside of the 33 allowable would indicate corruption. The fact that encoding/base32 allows a "special processing" character other than "=" doesn't matter, since the set of allowable characters is still 33.

@gopherbot
Copy link

CL https://golang.org/cl/47341 mentions this issue.

gopherbot pushed a commit that referenced this issue Jul 6, 2017
@zegl @bradfitz
encoding/base32: make NoPadding Encoding's DecodedLen return exact size
d1340ee 
CL 47341 added support for decoding non-padded messages. But DecodedLen
still returned a multiple of 5 for messages without a padding, even
though it is possible to calculate the len exactly when using NoPadding.

This change makes DecodedLen return the exact number of bytes that
will be written. A change to the decoding logic is also made so that it
can handle this case.

DecodedLen now has the same behaviour as DecodedLen in encoding/base64.

Fixes #20854

Change-Id: I729e0b1c0946c866fb675c854f835f366dd4b5a4
Reviewed-on: https://go-review.googlesource.com/47710
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
@gopherbot
Copy link

Change https://golang.org/cl/47750 mentions this issue: encoding/base32: make NoPadding Encoding's DecodedLen return exact size

@gopherbot
Copy link

Change https://golang.org/cl/47710 mentions this issue: encoding/base32: make NoPadding Encoding's DecodedLen return exact size

@biexiang
Copy link

go version go1.8.3 darwin/amd64

decode with pad "=" or "A" get illegal base32 data at input byte
"A" is in The Base 32 Alphabet
???

@ALTree
Copy link
Member

ALTree commented Nov 15, 2017

@biexiang if you believe you have found a bug, please open a new issue. Don't forget to include a reproducer.

@golang golang locked and limited conversation to collaborators Nov 15, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.9Maybe
Development

No branches or pull requests
6 participants
@bradfitz @zegl @ALTree @bobjalex @gopherbot @biexiang