Skip to content

Commit b58155f

Browse files
authored
🐛 Fix base64 _decode Double Padding Detection (#345)
### 🕓 Changelog This PR fixes incorrect padding detection in the `base64` `_decode` function. The logic incorrectly identified `/` or `_` (index `63`) as the padding character instead of `=` (index `64`), and checked single padding before double padding. This caused [Base64](https://en.wikipedia.org/wiki/Base64) strings with `/` or `_` in the third position (e.g. `AA/A` or `AA_A`) to be incorrectly truncated, and strings with double padding (`==`) to be mishandled. This patch corrects the index to `64`, reorders the conditional checks to handle double padding before single padding, and adds an assertion to reject invalid padding combinations. Since the `base64` contract is not used in production, this bug poses _no_ security risk. Thanks to @Leminkay for identifying this bug! --------- Signed-off-by: Pascal Marco Caversaccio <pascal.caversaccio@hotmail.ch>
1 parent 1548dab commit b58155f

File tree

7 files changed

+1347
-1300
lines changed

7 files changed

+1347
-1300
lines changed

.gas-snapshot

Lines changed: 588 additions & 586 deletions
Large diffs are not rendered by default.

.gas-snapshot-venom

Lines changed: 702 additions & 700 deletions
Large diffs are not rendered by default.

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@
77
- **Utility Functions**
88
- [`eip7702_utils`](https://github.com/pcaversaccio/snekmate/blob/v0.1.3/src/snekmate/utils/eip7702_utils.vy): Add [EIP-7702](https://eips.ethereum.org/EIPS/eip-7702)-based utility functions. ([#335](https://github.com/pcaversaccio/snekmate/pull/335))
99

10+
### 🐛 Bug Fixes
11+
12+
- **Utility Functions**
13+
- [`base64`](https://github.com/pcaversaccio/snekmate/blob/v0.1.3/src/snekmate/utils/base64.vy): Fix `base64` `_decode` double padding detection. ([#345](https://github.com/pcaversaccio/snekmate/pull/345))
14+
1015
### 🔖 Release Management
1116

1217
- Use trusted publishing for `npm` package. ([#341](https://github.com/pcaversaccio/snekmate/pull/341))

foundry.lock

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
"lib/openzeppelin-contracts": {
3333
"branch": {
3434
"name": "master",
35-
"rev": "04e683b550b496bfbf471ed960b7feed25b47669"
35+
"rev": "eb97fd3f153589bc4d7cd0c512956b18aa844cfd"
3636
}
3737
},
3838
"lib/prb-test": {

lib/openzeppelin-contracts

src/snekmate/utils/base64.vy

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -215,14 +215,15 @@ def _decode(data: String[_DATA_OUTPUT_BOUND], base64_url: bool) -> DynArray[Byte
215215
b2: bytes1 = convert(convert((chunk_bytes >> 8) & 255, uint8), bytes1)
216216
b3: bytes1 = convert(convert(chunk_bytes & 255, uint8), bytes1)
217217

218-
# Case 1: padding of "=" as part of the
218+
# Case 1: padding of "==" as part of the
219219
# encoded input.
220-
if c4 == 64:
221-
result.append(concat(b1, b2, x"00"))
222-
# Case 2: padding of "==" as part of the
223-
# encoded input.
224-
elif c3 == 63:
220+
if c3 == 64:
221+
assert c4 == 64, "base64: invalid padding"
225222
result.append(concat(b1, x"0000"))
223+
# Case 2: padding of "=" as part of the
224+
# encoded input.
225+
elif c4 == 64:
226+
result.append(concat(b1, b2, x"00"))
226227
# Case 3: no padding as part of the encoded
227228
# input.
228229
else:
@@ -252,14 +253,15 @@ def _decode(data: String[_DATA_OUTPUT_BOUND], base64_url: bool) -> DynArray[Byte
252253
b2: bytes1 = convert(convert((chunk_bytes >> 8) & 255, uint8), bytes1)
253254
b3: bytes1 = convert(convert(chunk_bytes & 255, uint8), bytes1)
254255

255-
# Case 1: padding of "=" as part of the
256+
# Case 1: padding of "==" as part of the
256257
# encoded input.
257-
if c4 == 64:
258-
result.append(concat(b1, b2, x"00"))
259-
# Case 2: padding of "==" as part of the
260-
# encoded input.
261-
elif c3 == 63:
258+
if c3 == 64:
259+
assert c4 == 64, "base64: invalid padding"
262260
result.append(concat(b1, x"0000"))
261+
# Case 2: padding of "=" as part of the
262+
# encoded input.
263+
elif c4 == 64:
264+
result.append(concat(b1, b2, x"00"))
263265
# Case 3: no padding as part of the encoded
264266
# input.
265267
else:

test/utils/Base64.t.sol

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,11 +208,47 @@ contract Base64Test is PRBTest {
208208
assertEq(string(returnDataUrl), text);
209209
}
210210

211+
function testDecodeIndex63NotTreatedAsPadding() public {
212+
string memory text1 = "AA/A";
213+
string memory data1 = "QUEvQQ==";
214+
bytes[] memory outputStd1 = base64.decode(data1, false);
215+
bytes[] memory outputUrl1 = base64.decode(data1, true);
216+
bytes memory returnDataStd1 = bytes.concat(outputStd1[0], outputStd1[1]);
217+
bytes memory returnDataUrl1 = bytes.concat(outputUrl1[0], outputUrl1[1]);
218+
/**
219+
* @dev We remove the two trailing zero bytes that stem from
220+
* the padding to ensure byte-level equality.
221+
*/
222+
assertEq(string(returnDataStd1.slice(0, returnDataStd1.length - 2)), text1);
223+
assertEq(string(returnDataUrl1.slice(0, returnDataUrl1.length - 2)), text1);
224+
225+
string memory text2 = "AA_A";
226+
string memory data2 = "QUFfQQ==";
227+
bytes[] memory outputStd2 = base64.decode(data2, false);
228+
bytes[] memory outputUrl2 = base64.decode(data2, true);
229+
bytes memory returnDataStd2 = bytes.concat(outputStd2[0], outputStd2[1]);
230+
bytes memory returnDataUrl2 = bytes.concat(outputUrl2[0], outputUrl2[1]);
231+
/**
232+
* @dev We remove the two trailing zero bytes that stem from
233+
* the padding to ensure byte-level equality.
234+
*/
235+
assertEq(string(returnDataStd2.slice(0, returnDataStd2.length - 2)), text2);
236+
assertEq(string(returnDataUrl2.slice(0, returnDataUrl2.length - 2)), text2);
237+
}
238+
211239
function testDataLengthMismatch() public {
212240
string memory data = "W11jI";
213241
vm.expectRevert(bytes("base64: length mismatch"));
214242
base64.decode(data, false);
215243
vm.expectRevert(bytes("base64: length mismatch"));
216244
base64.decode(data, true);
217245
}
246+
247+
function testInvalidPadding() public {
248+
string memory data = "TQ=u";
249+
vm.expectRevert(bytes("base64: invalid padding"));
250+
base64.decode(data, false);
251+
vm.expectRevert(bytes("base64: invalid padding"));
252+
base64.decode(data, true);
253+
}
218254
}

0 commit comments

Comments
 (0)