As we can see the intended arranging is first 2 single bytes for the clipboard type and use ether flag, then 30 empty bytes of padding to reach 32 bytes, and then the array of bytes32 should start.
In the beggining we can see that the first 32 bytes should contain the length of the array (how many bytes32 are stored in the array) and after that each of these elements.
If we look at the coded implementation of how the smart contracts does to extract all this data we see the following:

function decode(

bytes memory clipboard

)

internal

pure

returns (bytes1 typeId, uint256 etherValue, bytes32[] memory returnPasteParams)

{

typeId = clipboard[0];

if (typeId == 0x01) {

returnPasteParams = new bytes32[](1);

returnPasteParams[0] = abi.decode(clipboard, (bytes32));

} else if (typeId == 0x02) {

(, returnPasteParams) = abi.decode(clipboard, (bytes2, bytes32[]));

}

bytes1 useEther = clipboard[1];

if (useEther == 0x01) {

etherValue = clipboard.toUint256(clipboard.length - 32);

}

As we can see, when the typeId is 0x02, it abi.decode the whole clipboard into a bytes2 (which does not take the information because it already got it) and then it decodes the rest into a dynamic array of bytes32.
The problem is that the encoding of a dynamic array does not match with the docs. In the docs, to encode the dynamic array of bytes32, it uses 32 bytes at the begging to store the length of the array, and then the elements next to it.
However, the solidity encoding uses first 32 bytes to point where the array is stored, and then in this pointed location there are 32 bytes to store the length and the elements next to it.

The risk of these discrepancies between the docs and the coded implementation is that someone would follow the clipboard arranging of the docs and could get an unexpected result in the smart contract that would not revert.

Proof of Concept:

contract TestDecoder is Test {

MockDecoder public decoder;

function setUp() public {

decoder = new MockDecoder();

}

function test_badClipboadArranging() public {

// 02 Type (1 byte)

// 01 Use Ether Flag (1 byte)

// 000000000000000000000000000000000000000000000000000000000000 Padding (30 bytes)

// 0000000000000000000000000000000000000000000000000000000000000004 PasteParams.length (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000a PasteParams[0] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000b PasteParams[1] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000c PasteParams[2] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000d PasteParams[3] (32 bytes)

// 0000000000000000000000000000000000000000000000000000000000000005 Ether Value (32 bytes)

bytes memory clipboard = hex"02010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000d0000000000000000000000000000000000000000000000000000000000000005";

(bytes1 typeId, uint256 etherValue, bytes32[] memory returnPasteParams) = decoder.decode(clipboard);

// The typeId and ether value are extracted properly

assertEq(typeId, bytes1(0x02));

assertEq(etherValue, 5);

// However, the array of bytes32 is extracted wrong and decoding does not even revert, hence will return wrong values

// The returned array in this case is an empty array

assertEq(returnPasteParams, new bytes32[](0));

}

function test_goodClipboadArranging() public {

// 02 Type (1 byte)

// 01 Use Ether Flag (1 byte)

// 000000000000000000000000000000000000000000000000000000000000 Padding (30 bytes)

// 0000000000000000000000000000000000000000000000000000000000000040 PasteParams.pointer (32 bytes)

// 0000000000000000000000000000000000000000000000000000000000000004 PasteParams.length (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000a PasteParams[0] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000b PasteParams[1] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000c PasteParams[2] (32 bytes)

// 000000000000000000000000000000000000000000000000000000000000000d PasteParams[3] (32 bytes)

// 0000000000000000000000000000000000000000000000000000000000000005 Ether Value (32 bytes)

bytes memory clipboard = hex"020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000d0000000000000000000000000000000000000000000000000000000000000005";

(bytes1 typeId, uint256 etherValue, bytes32[] memory returnPasteParams) = decoder.decode(clipboard);

// All properties are extracted properly

assertEq(typeId, bytes1(0x02));

assertEq(etherValue, 5);

bytes32[] memory expectedResults = new bytes32[](4);

expectedResults[0] = bytes32(0x000000000000000000000000000000000000000000000000000000000000000a);

expectedResults[1] = bytes32(0x000000000000000000000000000000000000000000000000000000000000000b);

expectedResults[2] = bytes32(0x000000000000000000000000000000000000000000000000000000000000000c);

expectedResults[3] = bytes32(0x000000000000000000000000000000000000000000000000000000000000000d);

assertEq(returnPasteParams, expectedResults);

}

contract MockDecoder {

function decode(bytes memory clipboard) external pure returns(bytes1 typeId, uint256 etherValue, bytes32[] memory returnPasteParams){

typeId = clipboard[0];

if (typeId == 0x01) {

returnPasteParams = new bytes32[](1);

returnPasteParams[0] = abi.decode(clipboard, (bytes32));

} else if (typeId == 0x02) {

(, returnPasteParams) = abi.decode(clipboard, (bytes2, bytes32[]));

}

bytes1 useEther = clipboard[1];

if (useEther == 0x01) {

etherValue = toUint256(clipboard, clipboard.length - 32);

}

function toUint256(bytes memory _bytes, uint256 _start) internal pure returns (uint256) {

require(_start + 32 >= _start, "toUint256_overflow");

require(_bytes.length >= _start + 32, "toUint256_outOfBounds");

uint256 tempUint;

assembly {

tempUint := mload(add(add(_bytes, 0x20), _start))

}

return tempUint;

}

Traces of running both tests:

Ran 2 tests for test/Counter.t.sol:TestDecoder

[PASS] test_badClipboadArranging() (gas: 12686)

Traces:

[12686] TestDecoder::test_badClipboadArranging()

├─ [2036] MockDecoder::decode(0x02010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000d0000000000000000000000000000000000000000000000000000000000000005) [staticcall]

│ └─ ← [Return] 0x0200000000000000000000000000000000000000000000000000000000000000, 5, []

├─ [0] VM::assertEq(0x0200000000000000000000000000000000000000000000000000000000000000, 0x0200000000000000000000000000000000000000000000000000000000000000) [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertEq(5, 5) [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertEq([], []) [staticcall]

│ └─ ← [Return]

└─ ← [Stop]

[PASS] test_goodClipboadArranging() (gas: 14773)

Traces:

[14773] TestDecoder::test_goodClipboadArranging()

├─ [2675] MockDecoder::decode(0x020100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000a000000000000000000000000000000000000000000000000000000000000000b000000000000000000000000000000000000000000000000000000000000000c000000000000000000000000000000000000000000000000000000000000000d0000000000000000000000000000000000000000000000000000000000000005) [staticcall]

│ └─ ← [Return] 0x0200000000000000000000000000000000000000000000000000000000000000, 5, [0x000000000000000000000000000000000000000000000000000000000000000a, 0x000000000000000000000000000000000000000000000000000000000000000b, 0x000000000000000000000000000000000000000000000000000000000000000c, 0x000000000000000000000000000000000000000000000000000000000000000d]

├─ [0] VM::assertEq(0x0200000000000000000000000000000000000000000000000000000000000000, 0x0200000000000000000000000000000000000000000000000000000000000000) [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertEq(5, 5) [staticcall]

│ └─ ← [Return]

├─ [0] VM::assertEq([0x000000000000000000000000000000000000000000000000000000000000000a, 0x000000000000000000000000000000000000000000000000000000000000000b, 0x000000000000000000000000000000000000000000000000000000000000000c, 0x000000000000000000000000000000000000000000000000000000000000000d], [0x000000000000000000000000000000000000000000000000000000000000000a, 0x000000000000000000000000000000000000000000000000000000000000000b, 0x000000000000000000000000000000000000000000000000000000000000000c, 0x000000000000000000000000000000000000000000000000000000000000000d]) [staticcall]

│ └─ ← [Return]

└─ ← [Stop]

Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 953.21µs (193.68µs CPU time)

Ran 1 test suite in 3.16ms (953.21µs CPU time): 2 tests passed, 0 failed, 0 skipped (2 total tests)

We can clearly see that if users follow the docs arranging he will not get the expected result and the function will not even revert, that means that the transaction will be executed but with the wrong paste parameters.

Impact

High, the functionality does not work as exposed in the docs. Moreover, users can get unexpected results due to wrong decoding of the clipboard.

Tools Used

Manual review

Recommendations

There are 2 possible solutions:
1- Change docs to support the abi encoding accordingly by supporting the pointer position
2- Change the code to decode properly the data from the clipboard accordingly to the docs:

function decode(bytes memory clipboard) external pure returns(bytes1 typeId, uint256 etherValue, bytes32[] memory returnPasteParams){

typeId = clipboard[0];

if (typeId == 0x01) {

returnPasteParams = new bytes32[](1);

returnPasteParams[0] = abi.decode(clipboard, (bytes32));

} else if (typeId == 0x02) {

- (, returnPasteParams) = abi.decode(clipboard, (bytes2, bytes32[]));

+ uint256 length;

+ assembly {

+ // the length of the array should be stored in the second word

+ length := mload(add(clipboard, 64))

+ }

+ returnPasteParams = new bytes32[](4);

+ for(uint256 i; i < length; i++){

+ bytes32 returnParamI;

+ assembly {

+ returnParamI := mload(add(clipboard, add(64, mul(add(i, 1), 32))))

+ }

+ returnPasteParams[i] = returnParamI;

+ }

}

bytes1 useEther = clipboard[1];

if (useEther == 0x01) {

etherValue = toUint256(clipboard, clipboard.length - 32);

}

With this new implementation, if we run the previous test with the clipboard arranging accordingly to the docs, the returned array is fine as the user would expect: