CVE-2024-27094: Insufficient Information (NVD-CWE-noinfo)
value: OpenZeppelin Contracts is a library for secure smart contract development. The `Base64.encode` function encodes a `bytes` input by iterating over it in chunks of 3 bytes. When this input is not a multiple of 3, the last iteration may read parts of the memory that are beyond the input buffer. The vulnerability is fixed in 5.0.2 and 4.9.6.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-21T02:52Z
Repository: https://github.com/OpenZeppelin/openzeppelin-contracts
Language: Other
Files changed: .changeset/warm-geese-dance.md, contracts/mocks/Base64Dirty.sol, contracts/utils/Base64.sol, test/utils/Base64.test.js
Commit date: 2024-02-29 10:03:32 -0600
Commit message: Merge pull request from GHSA-9vx6-7xxf-x967

* add tests for the encode reads dirty data issue

* Fix the encode reads dirty data issue

* add changeset

* trigger the issue without assembly

* rename mock

* gas optimization

* Apply suggestions from code review

Co-authored-by: Ernesto García <ernestognw@gmail.com>

* alternative fix: cheaper

* update comment

* fix lint

---------

Co-authored-by: Ernesto García <ernestognw@gmail.com>
Diff: diff --git a/.changeset/warm-geese-dance.md b/.changeset/warm-geese-dance.md
new file mode 100644
index 000000000..5deb7a3e6
--- /dev/null
+++ b/.changeset/warm-geese-dance.md
@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`Base64`: Fix issue where dirty memory located just after the input buffer is affecting the result.
diff --git a/contracts/mocks/Base64Dirty.sol b/contracts/mocks/Base64Dirty.sol
new file mode 100644
index 000000000..238bd26a3
--- /dev/null
+++ b/contracts/mocks/Base64Dirty.sol
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.20;
+
+import {Base64} from "../utils/Base64.sol";
+
+contract Base64Dirty {
+    struct A {
+        uint256 value;
+    }
+
+    function encode(bytes memory input) public pure returns (string memory) {
+        A memory unused = A({value: type(uint256).max});
+        // To silence warning
+        unused;
+
+        return Base64.encode(input);
+    }
+}
diff --git a/contracts/utils/Base64.sol b/contracts/utils/Base64.sol
index 3f2d7c134..b7c748667 100644
--- a/contracts/utils/Base64.sol
+++ b/contracts/utils/Base64.sol
@@ -56,30 +56,36 @@ library Base64 {
         assembly {
             // Prepare the lookup table (skip the first "length" byte)
             let tablePtr := add(table, 1)
 
             // Prepare result pointer, jump over length
-            let resultPtr := add(result, 32)
+            let resultPtr := add(result, 0x20)
+            let dataPtr := data
+            let endPtr := add(data, mload(data))
+
+            // In some cases, the last iteration will read bytes after the end of the data. We cache the value, and
+            // set it to zero to make sure no dirty bytes are read in that section.
+            let afterPtr := add(endPtr, 0x20)
+            let afterCache := mload(afterPtr)
+            mstore(afterPtr, 0x00)
 
             // Run over the input, 3 bytes at a time
             for {
-                let dataPtr := data
-                let endPtr := add(data, mload(data))
+
             } lt(dataPtr, endPtr) {
 
             } {
                 // Advance 3 bytes
                 dataPtr := add(dataPtr, 3)
                 let input := mload(dataPtr)
 
-                // To write each character, shift the 3 bytes (18 bits) chunk
+                // To write each character, shift the 3 byte (24 bits) chunk
                 // 4 times in blocks of 6 bits for each character (18, 12, 6, 0)
-                // and apply logical AND with 0x3F which is the number of
-                // the previous character in the ASCII table prior to the Base64 Table
-                // The result is then added to the table to get the character to write,
-                // and finally write it in the result pointer but with a left shift
-                // of 256 (1 byte) - 8 (1 ASCII char) = 248 bits
+                // and apply logical AND with 0x3F to bitmask the least significant 6 bits.
+                // Use this as an index into the lookup table, mload an entire word
+                // so the desired character is in the least significant byte, and
+                // mstore8 this least significant byte into the result and continue.
 
                 mstore8(resultPtr, mload(add(tablePtr, and(shr(18, input), 0x3F))))
                 resultPtr := add(resultPtr, 1) // Advance
 
                 mstore8(resultPtr, mload(add(tablePtr, and(shr(12, input), 0x3F))))
@@ -90,10 +96,13 @@ library Base64 {
 
                 mstore8(resultPtr, mload(add(tablePtr, and(input, 0x3F))))
                 resultPtr := add(resultPtr, 1) // Advance
             }
 
+            // Reset the value that was cached
+            mstore(afterPtr, afterCache)
+
             if withPadding {
                 // When data `bytes` is not exactly 3 bytes long
                 // it is padded with `=` characters at the end
                 switch mod(mload(data), 3)
                 case 1 {
diff --git a/test/utils/Base64.test.js b/test/utils/Base64.test.js
index 3f3e9fc8b..5c4274666 100644
--- a/test/utils/Base64.test.js
+++ b/test/utils/Base64.test.js
@@ -45,6 +45,15 @@ describe('Strings', function () {
         const buffer = Buffer.from(input, 'ascii');
         expect(await this.mock.$encodeURL(buffer)).to.equal(base64toBase64Url(ethers.encodeBase64(buffer)));
         expect(await this.mock.$encodeURL(buffer)).to.equal(expected);
       });
   });
+
+  it('Encode reads beyond the input buffer into dirty memory', async function () {
+    const mock = await ethers.deployContract('Base64Dirty');
+    const buffer32 = ethers.id('example');
+    const buffer31 = buffer32.slice(0, -2);
+
+    expect(await mock.encode(buffer31)).to.equal(ethers.encodeBase64(buffer31));
+    expect(await mock.encode(buffer32)).to.equal(ethers.encodeBase64(buffer32));
+  });
 });

Vulnerable code:             let resultPtr := add(result, 32)
                let dataPtr := data
                let endPtr := add(data, mload(data))
                // To write each character, shift the 3 bytes (18 bits) chunk
                // and apply logical AND with 0x3F which is the number of
                // the previous character in the ASCII table prior to the Base64 Table
                // The result is then added to the table to get the character to write,
                // and finally write it in the result pointer but with a left shift
                // of 256 (1 byte) - 8 (1 ASCII char) = 248 bits
Fixed code: ---
'openzeppelin-solidity': patch
---

`Base64`: Fix issue where dirty memory located just after the input buffer is affecting the result.
// SPDX-License-Identifier: MIT

pragma solidity ^0.8.20;

import {Base64} from "../utils/Base64.sol";

contract Base64Dirty {
    struct A {
        uint256 value;
    }

    function encode(bytes memory input) public pure returns (string memory) {
        A memory unused = A({value: type(uint256).max});
        // To silence warning
        unused;

        return Base64.encode(input);
    }
}
            let resultPtr := add(result, 0x20)
            let dataPtr := data
            let endPtr := add(data, mload(data))

            // In some cases, the last iteration will read bytes after the end of the data. We cache the value, and
            // set it to zero to make sure no dirty bytes are read in that section.
            let afterPtr := add(endPtr, 0x20)
            let afterCache := mload(afterPtr)
            mstore(afterPtr, 0x00)

                // To write each character, shift the 3 byte (24 bits) chunk
                // and apply logical AND with 0x3F to bitmask the least significant 6 bits.
                // Use this as an index into the lookup table, mload an entire word
                // so the desired character is in the least significant byte, and
                // mstore8 this least significant byte into the result and continue.
            // Reset the value that was cached
            mstore(afterPtr, afterCache)


  it('Encode reads beyond the input buffer into dirty memory', async function () {
    const mock = await ethers.deployContract('Base64Dirty');
    const buffer32 = ethers.id('example');
    const buffer31 = buffer32.slice(0, -2);

    expect(await mock.encode(buffer31)).to.equal(ethers.encodeBase64(buffer31));
    expect(await mock.encode(buffer32)).to.equal(ethers.encodeBase64(buffer32));
  });