Skip to content

Reject non-canonical trailing bits when decoding in BaseEncoding #8746

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.

Sign up for GitHub

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

Open
arshsmith1 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reject non-canonical trailing bits when decoding in BaseEncoding #8746

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions android/guava-tests/test/com/google/common/io/BaseEncodingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ public void testBase64InvalidDecodings() {
assertFailsToDecode(base64(), "?", "Invalid input length 1");
}

public void testBase64RejectsNonCanonicalPaddingBits() {
// Bits of the final character that are not part of any decoded byte must be zero (RFC 4648
// section 3.5); otherwise many distinct strings decode to the same bytes.
assertFailsToDecode(base64(), "AB==", "Unused bits set in encoded input");
assertFailsToDecode(base64(), "QE==", "Unused bits set in encoded input");
assertFailsToDecode(base64(), "AAB=", "Unused bits set in encoded input");
// The canonical siblings still decode.
testDecodesByBytes(base64(), "AA==", new byte[] {0x00});
testDecodesByBytes(base64(), "QA==", new byte[] {0x40});
testDecodesByBytes(base64(), "AAA=", new byte[] {0x00, 0x00});
}

public void testBase64CannotUpperCase() {
assertThrows(IllegalStateException.class, () -> base64().upperCase());
}
Expand Down Expand Up @@ -226,6 +238,10 @@ public void testBase64UrlInvalidDecodings() {
assertFailsToDecode(base64Url(), "/zzz", "Unrecognized character: /");
}

public void testBase64UrlRejectsNonCanonicalPaddingBits() {
assertFailsToDecode(base64Url(), "AB", "Unused bits set in encoded input");
}

public void testBase32() {
// The following test vectors are specified in RFC 4648 itself
testEncodingWithCasing(base32(), "", "");
Expand Down Expand Up @@ -285,6 +301,15 @@ public void testBase32InvalidDecodings() {
assertFailsToDecode(base32(), "?", "Invalid input length 1");
}

public void testBase32RejectsNonCanonicalPaddingBits() {
// Bits of the final character that are not part of any decoded byte must be zero (RFC 4648
// section 3.5); "MZXW7" and "MZXW6" would otherwise both decode to "foo".
assertFailsToDecode(base32(), "MZXW7===", "Unused bits set in encoded input");
assertFailsToDecode(base32(), "AAAB====", "Unused bits set in encoded input");
testDecodes(base32(), "MZXW6===", "foo");
testDecodesByBytes(base32(), "AAAA====", new byte[] {0x00, 0x00});
}

public void testBase32UpperCaseIsNoOp() {
assertThat(base32().upperCase()).isSameInstanceAs(base32());
}
Expand Down Expand Up @@ -353,6 +378,12 @@ public void testBase32HexInvalidDecodings() {
assertFailsToDecode(base32Hex(), "ABCDEF");
}

public void testBase32HexRejectsNonCanonicalPaddingBits() {
assertFailsToDecode(base32Hex(), "CPNMV===", "Unused bits set in encoded input");
assertFailsToDecode(base32Hex(), "0001====", "Unused bits set in encoded input");
testDecodes(base32Hex(), "CPNMU===", "foo");
}

public void testBase32HexUpperCaseIsNoOp() {
assertThat(base32Hex().upperCase()).isSameInstanceAs(base32Hex());
}
Expand Down
33 changes: 32 additions & 1 deletion android/guava/src/com/google/common/io/BaseEncoding.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,20 @@ public boolean canDecode(CharSequence chars) {
return false;
}
}
// Any bits of the final character not consumed by a decoded byte must be zero for a
// canonical encoding (RFC 4648 section 3.5).
int lastChunkChars = chars.length() % alphabet.charsPerChunk;
if (lastChunkChars != 0) {
int unusedBits = (lastChunkChars * alphabet.bitsPerChar) % 8;
try {
if ((alphabet.decode(chars.charAt(chars.length() - 1)) & ((1 << unusedBits) - 1)) != 0) {
return false;
}
} catch (DecodingException e) {
// Can't happen: the loop above already verified every character is decodable.
return false;
}
}
return true;
}

Expand All @@ -774,9 +788,15 @@ int decodeTo(byte[] target, CharSequence chars) throws DecodingException {
}
}
int minOffset = alphabet.bytesPerChunk * 8 - charsProcessed * alphabet.bitsPerChar;
for (int offset = (alphabet.bytesPerChunk - 1) * 8; offset >= minOffset; offset -= 8) {
int offset = (alphabet.bytesPerChunk - 1) * 8;
for (; offset >= minOffset; offset -= 8) {
target[bytesWritten++] = (byte) ((chunk >>> offset) & 0xFF);
}
// Any bits of the final character not consumed by an emitted byte must be zero for a
// canonical encoding (RFC 4648 section 3.5).
if ((chunk & ((1L << (offset + 8)) - 1)) != 0) {
throw new DecodingException("Unused bits set in encoded input");
}
}
return bytesWritten;
}
Expand All @@ -800,6 +820,11 @@ public int read() throws IOException {
if (!hitPadding && !alphabet.isValidPaddingStartPosition(readChars)) {
throw new DecodingException("Invalid input length " + readChars);
}
// Any trailing bits not consumed by an emitted byte must be zero for a canonical
// encoding (RFC 4648 section 3.5).
if ((bitBuffer & ((1 << bitBufferLength) - 1)) != 0) {
throw new DecodingException("Unused bits set in encoded input");
}
return -1;
}
readChars++;
Expand Down Expand Up @@ -1045,7 +1070,13 @@ int decodeTo(byte[] target, CharSequence chars) throws DecodingException {
if (i < chars.length()) {
chunk |= alphabet.decode(chars.charAt(i++));
target[bytesWritten++] = (byte) (chunk & 0xFF);
} else if ((chunk & 0xFF) != 0) {
// Unused bits of the third character must be zero (RFC 4648 section 3.5).
throw new DecodingException("Unused bits set in encoded input");
}
} else if ((chunk & 0xFFFF) != 0) {
// Unused bits of the second character must be zero (RFC 4648 section 3.5).
throw new DecodingException("Unused bits set in encoded input");
}
}
return bytesWritten;
Expand Down
31 changes: 31 additions & 0 deletions guava-tests/test/com/google/common/io/BaseEncodingTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,18 @@ public void testBase64InvalidDecodings() {
assertFailsToDecode(base64(), "?", "Invalid input length 1");
}

public void testBase64RejectsNonCanonicalPaddingBits() {
// Bits of the final character that are not part of any decoded byte must be zero (RFC 4648
// section 3.5); otherwise many distinct strings decode to the same bytes.
assertFailsToDecode(base64(), "AB==", "Unused bits set in encoded input");
assertFailsToDecode(base64(), "QE==", "Unused bits set in encoded input");
assertFailsToDecode(base64(), "AAB=", "Unused bits set in encoded input");
// The canonical siblings still decode.
testDecodesByBytes(base64(), "AA==", new byte[] {0x00});
testDecodesByBytes(base64(), "QA==", new byte[] {0x40});
testDecodesByBytes(base64(), "AAA=", new byte[] {0x00, 0x00});
}

public void testBase64CannotUpperCase() {
assertThrows(IllegalStateException.class, () -> base64().upperCase());
}
Expand Down Expand Up @@ -226,6 +238,10 @@ public void testBase64UrlInvalidDecodings() {
assertFailsToDecode(base64Url(), "/zzz", "Unrecognized character: /");
}

public void testBase64UrlRejectsNonCanonicalPaddingBits() {
assertFailsToDecode(base64Url(), "AB", "Unused bits set in encoded input");
}

public void testBase32() {
// The following test vectors are specified in RFC 4648 itself
testEncodingWithCasing(base32(), "", "");
Expand Down Expand Up @@ -285,6 +301,15 @@ public void testBase32InvalidDecodings() {
assertFailsToDecode(base32(), "?", "Invalid input length 1");
}

public void testBase32RejectsNonCanonicalPaddingBits() {
// Bits of the final character that are not part of any decoded byte must be zero (RFC 4648
// section 3.5); "MZXW7" and "MZXW6" would otherwise both decode to "foo".
assertFailsToDecode(base32(), "MZXW7===", "Unused bits set in encoded input");
assertFailsToDecode(base32(), "AAAB====", "Unused bits set in encoded input");
testDecodes(base32(), "MZXW6===", "foo");
testDecodesByBytes(base32(), "AAAA====", new byte[] {0x00, 0x00});
}

public void testBase32UpperCaseIsNoOp() {
assertThat(base32().upperCase()).isSameInstanceAs(base32());
}
Expand Down Expand Up @@ -353,6 +378,12 @@ public void testBase32HexInvalidDecodings() {
assertFailsToDecode(base32Hex(), "ABCDEF");
}

public void testBase32HexRejectsNonCanonicalPaddingBits() {
assertFailsToDecode(base32Hex(), "CPNMV===", "Unused bits set in encoded input");
assertFailsToDecode(base32Hex(), "0001====", "Unused bits set in encoded input");
testDecodes(base32Hex(), "CPNMU===", "foo");
}

public void testBase32HexUpperCaseIsNoOp() {
assertThat(base32Hex().upperCase()).isSameInstanceAs(base32Hex());
}
Expand Down
33 changes: 32 additions & 1 deletion guava/src/com/google/common/io/BaseEncoding.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,20 @@ public boolean canDecode(CharSequence chars) {
return false;
}
}
// Any bits of the final character not consumed by a decoded byte must be zero for a
// canonical encoding (RFC 4648 section 3.5).
int lastChunkChars = chars.length() % alphabet.charsPerChunk;
if (lastChunkChars != 0) {
int unusedBits = (lastChunkChars * alphabet.bitsPerChar) % 8;
try {
if ((alphabet.decode(chars.charAt(chars.length() - 1)) & ((1 << unusedBits) - 1)) != 0) {
return false;
}
} catch (DecodingException e) {
// Can't happen: the loop above already verified every character is decodable.
return false;
}
}
return true;
}

Expand All @@ -774,9 +788,15 @@ int decodeTo(byte[] target, CharSequence chars) throws DecodingException {
}
}
int minOffset = alphabet.bytesPerChunk * 8 - charsProcessed * alphabet.bitsPerChar;
for (int offset = (alphabet.bytesPerChunk - 1) * 8; offset >= minOffset; offset -= 8) {
int offset = (alphabet.bytesPerChunk - 1) * 8;
for (; offset >= minOffset; offset -= 8) {
target[bytesWritten++] = (byte) ((chunk >>> offset) & 0xFF);
}
// Any bits of the final character not consumed by an emitted byte must be zero for a
// canonical encoding (RFC 4648 section 3.5).
if ((chunk & ((1L << (offset + 8)) - 1)) != 0) {
throw new DecodingException("Unused bits set in encoded input");
}
}
return bytesWritten;
}
Expand All @@ -800,6 +820,11 @@ public int read() throws IOException {
if (!hitPadding && !alphabet.isValidPaddingStartPosition(readChars)) {
throw new DecodingException("Invalid input length " + readChars);
}
// Any trailing bits not consumed by an emitted byte must be zero for a canonical
// encoding (RFC 4648 section 3.5).
if ((bitBuffer & ((1 << bitBufferLength) - 1)) != 0) {
throw new DecodingException("Unused bits set in encoded input");
}
return -1;
}
readChars++;
Expand Down Expand Up @@ -1045,7 +1070,13 @@ int decodeTo(byte[] target, CharSequence chars) throws DecodingException {
if (i < chars.length()) {
chunk |= alphabet.decode(chars.charAt(i++));
target[bytesWritten++] = (byte) (chunk & 0xFF);
} else if ((chunk & 0xFF) != 0) {
// Unused bits of the third character must be zero (RFC 4648 section 3.5).
throw new DecodingException("Unused bits set in encoded input");
}
} else if ((chunk & 0xFFFF) != 0) {
// Unused bits of the second character must be zero (RFC 4648 section 3.5).
throw new DecodingException("Unused bits set in encoded input");
}
}
return bytesWritten;
Expand Down
Loading