Skip to content

Commit 2492017

Browse files
Transpile a6286d0f
1 parent a40cb0b commit 2492017

File tree

6 files changed

+75
-12
lines changed

6 files changed

+75
-12
lines changed

.changeset/warm-geese-dance.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'openzeppelin-solidity': patch
3+
---
4+
5+
`Base64`: Fix issue where dirty memory located just after the input buffer is affecting the result.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
// SPDX-License-Identifier: MIT
2+
3+
pragma solidity ^0.8.0;
4+
5+
import {Base64Upgradeable} from "../utils/Base64Upgradeable.sol";
6+
import {Initializable} from "../proxy/utils/Initializable.sol";
7+
8+
contract Base64DirtyUpgradeable is Initializable {
9+
struct A {
10+
uint256 value;
11+
}
12+
13+
function __Base64Dirty_init() internal onlyInitializing {
14+
}
15+
16+
function __Base64Dirty_init_unchained() internal onlyInitializing {
17+
}
18+
function encode(bytes memory input) public pure returns (string memory) {
19+
A memory unused = A({value: type(uint256).max});
20+
// To silence warning
21+
unused;
22+
23+
return Base64Upgradeable.encode(input);
24+
}
25+
26+
/**
27+
* @dev This empty reserved space is put in place to allow future versions to add new
28+
* variables without shifting down storage in the inheritance chain.
29+
* See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
30+
*/
31+
uint256[50] private __gap;
32+
}

contracts/mocks/WithInit.sol

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,13 @@ contract Bytes32ArraysMockUpgradeableWithInit is Bytes32ArraysMockUpgradeable {
147147
__Bytes32ArraysMock_init(array);
148148
}
149149
}
150+
import "./Base64DirtyUpgradeable.sol";
151+
152+
contract Base64DirtyUpgradeableWithInit is Base64DirtyUpgradeable {
153+
constructor() payable initializer {
154+
__Base64Dirty_init();
155+
}
156+
}
150157
import "./CallReceiverMockUpgradeable.sol";
151158

152159
contract CallReceiverMockUpgradeableWithInit is CallReceiverMockUpgradeable {

contracts/utils/Base64Upgradeable.sol

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,26 +41,32 @@ library Base64Upgradeable {
4141
let tablePtr := add(table, 1)
4242

4343
// Prepare result pointer, jump over length
44-
let resultPtr := add(result, 32)
44+
let resultPtr := add(result, 0x20)
45+
let dataPtr := data
46+
let endPtr := add(data, mload(data))
47+
48+
// In some cases, the last iteration will read bytes after the end of the data. We cache the value, and
49+
// set it to zero to make sure no dirty bytes are read in that section.
50+
let afterPtr := add(endPtr, 0x20)
51+
let afterCache := mload(afterPtr)
52+
mstore(afterPtr, 0x00)
4553

4654
// Run over the input, 3 bytes at a time
4755
for {
48-
let dataPtr := data
49-
let endPtr := add(data, mload(data))
56+
5057
} lt(dataPtr, endPtr) {
5158

5259
} {
5360
// Advance 3 bytes
5461
dataPtr := add(dataPtr, 3)
5562
let input := mload(dataPtr)
5663

57-
// To write each character, shift the 3 bytes (18 bits) chunk
64+
// To write each character, shift the 3 byte (24 bits) chunk
5865
// 4 times in blocks of 6 bits for each character (18, 12, 6, 0)
59-
// and apply logical AND with 0x3F which is the number of
60-
// the previous character in the ASCII table prior to the Base64 Table
61-
// The result is then added to the table to get the character to write,
62-
// and finally write it in the result pointer but with a left shift
63-
// of 256 (1 byte) - 8 (1 ASCII char) = 248 bits
66+
// and apply logical AND with 0x3F to bitmask the least significant 6 bits.
67+
// Use this as an index into the lookup table, mload an entire word
68+
// so the desired character is in the least significant byte, and
69+
// mstore8 this least significant byte into the result and continue.
6470

6571
mstore8(resultPtr, mload(add(tablePtr, and(shr(18, input), 0x3F))))
6672
resultPtr := add(resultPtr, 1) // Advance
@@ -75,6 +81,9 @@ library Base64Upgradeable {
7581
resultPtr := add(resultPtr, 1) // Advance
7682
}
7783

84+
// Reset the value that was cached
85+
mstore(afterPtr, afterCache)
86+
7887
// When data `bytes` is not exactly 3 bytes long
7988
// it is padded with `=` characters at the end
8089
switch mod(mload(data), 3)

package-lock.json

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/utils/Base64.test.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
const { expect } = require('chai');
22

33
const Base64 = artifacts.require('$Base64');
4+
const Base64Dirty = artifacts.require('$Base64Dirty');
45

5-
contract('Strings', function () {
6+
contract('Base64', function () {
67
beforeEach(async function () {
78
this.base64 = await Base64.new();
89
});
@@ -30,4 +31,13 @@ contract('Strings', function () {
3031
expect(await this.base64.$encode([])).to.equal('');
3132
});
3233
});
34+
35+
it('Encode reads beyond the input buffer into dirty memory', async function () {
36+
const mock = await Base64Dirty.new();
37+
const buffer32 = Buffer.from(web3.utils.soliditySha3('example').replace(/0x/, ''), 'hex');
38+
const buffer31 = buffer32.slice(0, -2);
39+
40+
expect(await mock.encode(buffer31)).to.equal(buffer31.toString('base64'));
41+
expect(await mock.encode(buffer32)).to.equal(buffer32.toString('base64'));
42+
});
3343
});

0 commit comments

Comments
 (0)