Audit Fixes for RLP library on Broadcaster Audit

.changeset/slow-lemons-rest.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+clarify natspec for encode functions on RLP library

contracts/utils/RLP.sol

@@ -115,17 +115,22 @@ library RLP {
     function encode(bool input) internal pure returns (bytes memory result) {
         assembly ("memory-safe") {
             result := mload(0x40)
-            mstore(result, 0x01) // length of the encoded data: 1 byte
+            mstore(result, 1) // length of the encoded data: 1 byte
             mstore8(add(result, 0x20), add(mul(iszero(input), 0x7f), 1)) // input
             mstore(0x40, add(result, 0x21)) // reserve memory
         }
     }
 
-    /// @dev Encode an address as RLP.
+    /**
+     * @dev Encode an address as RLP.
+     *
+     * The address is encoded with its leading zeros (if it has any). If someone wants to encode the address as a scalar,
+     * they can cast it to an uint256 and then call the corresponding {encode} function.
+     */
     function encode(address input) internal pure returns (bytes memory result) {
         assembly ("memory-safe") {
             result := mload(0x40)
-            mstore(result, 0x15) // length of the encoded data: 1 (prefix) + 0x14 (address)
+            mstore(result, 21) // length of the encoded data: 1 (prefix) + 0x14 (address)
             mstore(add(result, 0x20), or(shl(248, 0x94), shl(88, input))) // prefix (0x94 = SHORT_OFFSET + 0x14) + input
             mstore(0x40, add(result, 0x35)) // reserve memory
         }
@@ -167,13 +172,17 @@ library RLP {
         return encode(bytes(input));
     }
 
-    /// @dev Encode an array of bytes as RLP.
+    /**
+     * @dev Encode an array of bytes as RLP.
+     * This function expects an array of already encoded bytes, not raw bytes.
+     * Users should call {encode} on each element of the array before calling it.
+     */
     function encode(bytes[] memory input) internal pure returns (bytes memory) {
         return _encode(input.concat(), LONG_OFFSET);
     }
 
     /// @dev Encode an encoder (list of bytes) as RLP
-    function encode(Encoder memory self) internal pure returns (bytes memory result) {
+    function encode(Encoder memory self) internal pure returns (bytes memory) {
         return _encode(self.acc.flatten(), LONG_OFFSET);
     }
 
@@ -241,7 +250,7 @@ library RLP {
         (uint256 offset, uint256 length, ItemType itemType) = _decodeLength(item);
         require(itemType == ItemType.Data, RLPInvalidEncoding());
 
-        // Length is checked by {toBytes}
+        // Length is checked by {slice}
         return item.slice(offset, length).toBytes();
     }
 
@@ -326,9 +335,7 @@ library RLP {
      * @dev Decodes an RLP `item`'s `length and type from its prefix.
      * Returns the offset, length, and type of the RLP item based on the encoding rules.
      */
-    function _decodeLength(
-        Memory.Slice item
-    ) private pure returns (uint256 _offset, uint256 _length, ItemType _itemtype) {
+    function _decodeLength(Memory.Slice item) private pure returns (uint256, uint256, ItemType) {
         uint256 itemLength = item.length();
 
         require(itemLength != 0, RLPInvalidEncoding());

test/utils/RLP.t.sol

@@ -132,4 +132,13 @@ contract RLPTest is Test {
 
         assertEq(RLP.encoder().push(b).push(a).push(u).encode(), RLP.encode(list));
     }
+
+    function testComputeCreateAddress(address deployer, uint256 nonce) external pure {
+        nonce = bound(nonce, 0, type(uint64).max);
+
+        assertEq(
+            address(uint160(uint256(keccak256(RLP.encoder().push(deployer).push(nonce).encode())))),
+            vm.computeCreateAddress(deployer, nonce)
+        );
+    }
 }

test/utils/RLP.test.js

@@ -2,6 +2,8 @@ const { ethers } = require('hardhat');
 const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
+const { MAX_UINT64 } = require('../helpers/constants');
+const { product } = require('../helpers/iterate');
 const { generators } = require('../helpers/random');
 
 async function fixture() {
@@ -33,11 +35,15 @@ describe('RLP', function () {
   });
 
   it('encode/decode addresses', async function () {
-    const addr = generators.address();
-    const expected = ethers.encodeRlp(addr);
-
-    await expect(this.mock.$encode_address(addr)).to.eventually.equal(expected);
-    await expect(this.mock.$decodeAddress(expected)).to.eventually.equal(addr);
+    for (const addr of [
+      ethers.ZeroAddress, // zero address
+      '0x0000F90827F1C53a10cb7A02335B175320002935', // address with leading zeros
+      generators.address(), // random address
+    ]) {
+      const expected = ethers.encodeRlp(addr);
+      await expect(this.mock.$encode_address(addr)).to.eventually.equal(expected);
+      await expect(this.mock.$decodeAddress(expected)).to.eventually.equal(addr);
+    }
   });
 
   it('encode/decode uint256', async function () {
@@ -117,6 +123,7 @@ describe('RLP', function () {
   it('encode/decode strings', async function () {
     for (const input of [
       '', // empty string
+      '000000000cat', // string with leading zeros
       'dog',
       'Lorem ipsum dolor sit amet, consectetur adipisicing elit',
     ]) {
@@ -146,4 +153,21 @@ describe('RLP', function () {
       await expect(this.mock.$decodeBytes(input)).to.be.revertedWithCustomError(this.mock, 'RLPInvalidEncoding');
     });
   });
+
+  it('RLP encoder predict create addresses', async function () {
+    for (const [from, nonce] of product(
+      [
+        ethers.ZeroAddress, // zero address
+        '0x0000F90827F1C53a10cb7A02335B175320002935', // address with heading zeros
+        generators.address(), // random address
+      ],
+      [0n, 1n, 42n, 65535n, MAX_UINT64],
+    )) {
+      await expect(
+        this.mock
+          .$encode_list([this.mock.$encode_address(from), this.mock.$encode_uint256(nonce)])
+          .then(encoded => ethers.getAddress(ethers.dataSlice(ethers.keccak256(encoded), 12))), // hash the encoded content, take the last 20 bytes and format as (checksummed) address
+      ).to.eventually.equal(ethers.getCreateAddress({ from, nonce }));
+    }
+  });
 });