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/base64: wrongly decodes bad base64 as valid #15656

Closed
harshavardhana opened this issue May 11, 2016 · 5 comments
Closed

encoding/base64: wrongly decodes bad base64 as valid #15656

harshavardhana opened this issue May 11, 2016 · 5 comments
Labels
FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.8Maybe

Comments

@harshavardhana
Copy link
Contributor

harshavardhana commented May 11, 2016 •
edited

Please answer these questions before submitting your issue. Thanks!

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

go1.6

  1. What operating system and processor architecture are you using (go env)?
    All operating systems.
  2. What did you do?
    https://play.golang.org/p/DiwWTDmGyC
package main

import (
    "encoding/base64"
    "encoding/hex"
    "fmt"
)

func main() {
    // Good base64.
    goodBytes, err := base64.StdEncoding.DecodeString("WvLTlMrX9NpYDQlEIFlnDA==")
    if err != nil {
        fmt.Println(err)
        return
    }
    // Bad base64.
    badBytes, err := base64.StdEncoding.DecodeString("WvLTlMrX9NpYDQlEIFlnDB==")
    if err != nil {
        fmt.Println(err)
        return
    }
    goodHex := hex.EncodeToString(goodBytes)
    badHex := hex.EncodeToString(badBytes)
    fmt.Println(goodHex)
    fmt.Println(badHex)

    goodBytes, err = hex.DecodeString(goodHex)
    if err != nil {
        fmt.Println(err)
        return
    }

    badBytes, err = hex.DecodeString(badHex)
    if err != nil {
        fmt.Println(err)
        return
    }

    goodBase64 := base64.StdEncoding.EncodeToString(goodBytes)
    badBase64 := base64.StdEncoding.EncodeToString(badBytes)
    fmt.Println(goodBase64)
    fmt.Println(badBase64)
}
  1. What did you expect to see?

Error for the second bad base64

    badBytes, err := base64.StdEncoding.DecodeString("WvLTlMrX9NpYDQlEIFlnDB==")

Here err should be non 'nil'.

An example of ruby decoder throwing ArgumentError for similar conditions under RFC4648.

require "base64"

good = Base64.strict_decode64("WvLTlMrX9NpYDQlEIFlnDA==") ## RFC4648
puts good.length

begin
  bad = Base64.strict_decode64("WvLTlMrX9NpYDQlEIFlnDB==") ## RFC4648
  puts bad.length
rescue ArgumentError
  puts "Invalid base64"
end
  1. What did you see instead?

Success for the second bad base64.
@harshavardhana harshavardhana mentioned this issue May 12, 2016
Closed
@kostya-sh
Copy link
Contributor

kostya-sh commented May 12, 2016 •
edited

Python 2.7.8 base64 module implements decoding the same way as encoding/base64 in Go:

>>> base64.b64decode("WvLTlMrX9NpYDQlEIFlnDB==")
'Z\xf2\xd3\x94\xca\xd7\xf4\xdaX\r\tD Yg\x0c'
>>> base64.b64decode("WvLTlMrX9NpYDQlEIFlnDA==")
'Z\xf2\xd3\x94\xca\xd7\xf4\xdaX\r\tD Yg\x0c'

RFC4648 allows this. The relevant section is 3.5 (https://www.ietf.org/rfc/rfc4648.txt):

The padding step in base 64 and base 32 encoding can, if improperly
implemented, lead to non-significant alterations of the encoded data.
For example, if the input is only one octet for a base 64 encoding,
then all six bits of the first symbol are used, but only the first
two bits of the next symbol are used. These pad bits MUST be set to
zero by conforming encoders, which is described in the descriptions
on padding below. If this property do not hold, there is no
canonical representation of base-encoded data, and multiple base-
encoded strings can be decoded to the same binary data. If this
property (and others discussed in this document) holds, a canonical
encoding is guaranteed.

In some environments, the alteration is critical and therefore
decoders MAY chose to reject an encoding if the pad bits have not
been set to zero. The specification referring to this may mandate a
specific behaviour.

@bradfitz bradfitz added this to the Go1.8Maybe milestone May 12, 2016
@bradfitz
Copy link
Contributor

I suppose we could have some opt-in mechanism to be stricter, per the spec's "The specification referring to this may mandate a specific behaviour." I could imagine security applications demanding more strictness, for example.

Maybe we add a method to base64.Encoding like:

// Strict returns a strict encoding of e. It does not modify e.
func (e *Encoding) Strict() *Encoding {
   ....
}

@bradfitz bradfitz changed the title encoding: base64 wrongly decodes bad base64 as valid. encoding/base64: wrongly decodes bad base64 as valid May 12, 2016
@xuyangkang
Copy link
Contributor

This sounds interesting to me. May I take over it?

@bradfitz
Copy link
Contributor

@xuyangkang, go for it.

@gopherbot
Copy link

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

@quentinmit quentinmit added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2016
@golang golang locked and limited conversation to collaborators Oct 12, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FeatureRequest FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.8Maybe
Development

No branches or pull requests
6 participants
@bradfitz @quentinmit @harshavardhana @xuyangkang @kostya-sh @gopherbot