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

encoding/base64: make decoding more lenient (?) #28205

Closed
koggle opened this issue Oct 15, 2018 · 6 comments

Comments

@koggle
Copy link

@koggle koggle commented Oct 15, 2018

Please answer these questions before submitting your issue. Thanks!

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

Does this issue reproduce with the latest release?

yes

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

win

What did you do?

package main
import (
"fmt"
"encoding/base64"
)
func main() {
fmt.Println(base64.RawStdEncoding.DecodeString("abc ")) //decoding error
}
If possible, provide a recipe for reproducing the error.
A complete runnable program is good.
A link on play.golang.org is best.

What did you expect to see?

//print [105,183] nil

What did you see instead?

//print [] illegal base64 data at input byte 3

@zegl

This comment has been minimized.

Copy link
Contributor

@zegl zegl commented Oct 15, 2018

@koggle Remove the trailing space in the DecodeString call, and it will work as expected. Whitespace is not a valid base64 character.

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Oct 15, 2018

The template is still not complete. Please re-read what @ianlancetaylor and myself requested in #28185.

In particular, fill in go version and go env, and clearly explain why you think this is broken. As far as I can see, this is working as intended, just like @zegl said.

@koggle

This comment has been minimized.

Copy link
Author

@koggle koggle commented Oct 15, 2018

This has nothing to do with go version and go env, thanks

@koggle

This comment has been minimized.

Copy link
Author

@koggle koggle commented Oct 15, 2018

Please refer to the code, thanks.

func (enc *Encoding) decodeQuantum(dst, src []byte, si int) (nsi, n int, err error) {
// Decode quantum using the base64 alphabet
var dbuf [4]byte
dinc, dlen := 3, 4

for j := 0; j < len(dbuf); j++ {
	if len(src) == si {
		switch {
		case j == 0:
			return si, 0, nil
		case j == 1, enc.padChar != NoPadding:
			return si, 0, CorruptInputError(si - j)
		}
		dinc, dlen = j-1, j
		break
	}
	in := src[si]
	si++

	out := enc.decodeMap[in]
	if out != 0xff {
		dbuf[j] = out
		continue
	}

	if in == '\n' || in == '\r' {
		j--
		continue
	}

	if rune(in) != enc.padChar {
		return si, 0, CorruptInputError(si - 1)
	}

	// We've reached the end and there's padding
	switch j {
	case 0, 1:
		// incorrect padding
		return si, 0, CorruptInputError(si - 1)
	case 2:
		// "==" is expected, the first "=" is already consumed.
		// skip over newlines
		for si < len(src) && (src[si] == '\n' || src[si] == '\r') {
			si++
		}
		if si == len(src) {
			// not enough padding
			return si, 0, CorruptInputError(len(src))
		}
		if rune(src[si]) != enc.padChar {
			// incorrect padding
			return si, 0, CorruptInputError(si - 1)
		}

		si++
	}

	// skip over newlines
	for si < len(src) && (src[si] == '\n' || src[si] == '\r') {
		si++
	}
	if si < len(src) {
		// trailing garbage
		err = CorruptInputError(si)
	}
	dinc, dlen = 3, j
	break
}

// Convert 4x 6bit source bytes into 3 bytes
val := uint(dbuf[0])<<18 | uint(dbuf[1])<<12 | uint(dbuf[2])<<6 | uint(dbuf[3])
dbuf[2], dbuf[1], dbuf[0] = byte(val>>0), byte(val>>8), byte(val>>16)
switch dlen {
case 4:
	dst[2] = dbuf[2]
	dbuf[2] = 0
	fallthrough
case 3:
	dst[1] = dbuf[1]
	if enc.strict && dbuf[2] != 0 {
		return si, 0, CorruptInputError(si - 1)
	}
	dbuf[1] = 0
	fallthrough
case 2:
	dst[0] = dbuf[0]
	if enc.strict && (dbuf[1] != 0 || dbuf[2] != 0) {
		return si, 0, CorruptInputError(si - 2)
	}
}
dst = dst[dinc:]

return si, dlen - 1, err

}

The above code is well written, but I simplified it and it is clearer.

func (enc *Encoding) decodeQuantum(dst, src []byte, si int) (nsi, n int, err error) {
// Decode quantum using the base64 alphabet
var dbuf [4]byte
dinc, dlen := 3, 4

for j := 0; j < len(dbuf); j++ {
	if len(src) != si {
		in := src[si]
		si++

		out := enc.decodeMap[in]
		if out != 0xff {
			dbuf[j] = out
			continue
		}

		if in == '\n' || in == '\r' {
			// skip over newlines
			j--
			continue
		}

		// fix bug, if we don't know the code is padding or nopading
		if rune(in) != enc.padChar {
			err = CorruptInputError(si - 1)
		}
	}


	
	// We've reached the end and there's padding
	// fix bug
	switch j {
	case 0, 1:
		// incorrect padding or "=="
		return si, 0, err
	case 2:
		// "==" is expected one byte
		dinc, dlen = 1, j
	case 3:
		// "=" is expected two byte
		dinc, dlen = 2, j
	}
	break
}

// Convert 4x 6bit source bytes into 3 bytes
val := uint(dbuf[0])<<18 | uint(dbuf[1])<<12 | uint(dbuf[2])<<6 | uint(dbuf[3])
dbuf[2], dbuf[1], dbuf[0] = byte(val>>0), byte(val>>8), byte(val>>16)
switch dlen {
case 4:
	dst[2] = dbuf[2]
	fallthrough
case 3:
	dst[1] = dbuf[1]
	fallthrough
case 2:
	dst[0] = dbuf[0]
}
dst = dst[dinc:]

return si, dlen - 1, err

}

// DecodedLen returns the maximum length in bytes of the decoded data
// corresponding to n bytes of base64-encoded data.
func (enc *Encoding) DecodedLen(n int) int {
// Unpadded data may end with partial block of 2-3 characters.
return n * 6 / 8
}

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Oct 15, 2018

Yes, the Go version and environment is relevant even when you think it isn't. It's just a matter of being clear so that maintainers and contributors don't have to spend extra time asking for extra information, just like I am doing right now.

Please don't dump large amounts of code - that is not helping clarify what the issue is or why you think it's a bug.

@FiloSottile FiloSottile changed the title fix base64 please encoding/base64: make decoding more lenient (?) Oct 15, 2018
@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 15, 2018

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this Nov 15, 2018
@golang golang locked and limited conversation to collaborators Nov 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
4 participants
You can’t perform that action at this time.