OpenZeppelin / openzeppelin-labs

A space for the community to interact and exchange ideas on the OpenZeppelin platform. Do not use in production!
https://openzeppelin.com/
MIT License
376 stars 116 forks source link

Proxy delegatecall for DYNAMIC array #137

Closed iAmMichaelConnor closed 2 years ago

iAmMichaelConnor commented 5 years ago

I can't get the Proxy contract to correctly do a delegatecall to a function which takes a dynamic array as its parameter.

Q: Is there a way to edit the Proxy's inline assembly code to correctly forward msg.data which contains a dynamic array? (I really need dynamic arrays to be passed to the contract).

I give a simple example of the problem below:

This is the function which I wish to call through the Proxy contract: function myDynamicFunction(uint[2][] myArray)

The delegatecall to this fails.

If I remove the 'dynamic' nature of the array, then the function is correctly executed. E.g.: function myDynamicFunction(uint[2][4] myArray) (fixed array size of 4).

The delegatecall to this works.

Here's what I think the problem is:

Say I want to pass the following array (an array of 4 arrays of 2 elements) to myDynamicFunction:

[["0x1111111111111111111111111111111111111111111111111111111111111111",
"0x2222222222222222222222222222222222222222222222222222222222222222"],
["0x3333333333333333333333333333333333333333333333333333333333333333",
"0x4444444444444444444444444444444444444444444444444444444444444444"],
["0x5555555555555555555555555555555555555555555555555555555555555555",
"0x6666666666666666666666666666666666666666666666666666666666666666"],
["0x7777777777777777777777777777777777777777777777777777777777777777",
"0x8888888888888888888888888888888888888888888888888888888888888888"]]

This is the correct msg.data of a dynamic array, which is passed into the Proxy:

0x
5a3d9eaf // (1) function signature
0000000000000000000000000000000000000000000000000000000000000020 // (2) the 'start' of the dynamic array's data starts after 32 bytes
0000000000000000000000000000000000000000000000000000000000000004 // (3) the dynamic array has 4 elements
1111111111111111111111111111111111111111111111111111111111111111 // (4) element 1 [0]
2222222222222222222222222222222222222222222222222222222222222222 // (5) element 1 [1]
3333333333333333333333333333333333333333333333333333333333333333 // (6) element 2 [0]
4444444444444444444444444444444444444444444444444444444444444444 // (7) element 2 [1]
5555555555555555555555555555555555555555555555555555555555555555 // (8) element 3 [0]
6666666666666666666666666666666666666666666666666666666666666666 // (9) element 3 [1]
7777777777777777777777777777777777777777777777777777777777777777 // (10) element 4 [0]
8888888888888888888888888888888888888888888888888888888888888888 // (11) element 4 [1]

However, I believe the delegatecall reformats this message data into an incorrect format as follows (see the solidity code further below to see how I deduced this in remix):

0x
d608741f  // (1) incorrect function signature?
// then the array being passed as though it is of fixed size (rather than a dynamic encoding):
1111111111111111111111111111111111111111111111111111111111111111 // (2) element 1 [0]
2222222222222222222222222222222222222222222222222222222222222222 // (3) element 1 [1]
3333333333333333333333333333333333333333333333333333333333333333 // (4) element 2 [0]
4444444444444444444444444444444444444444444444444444444444444444 // (5) element 2 [1]
5555555555555555555555555555555555555555555555555555555555555555 // (6) element 3 [0]
6666666666666666666666666666666666666666666666666666666666666666 // (7) element 3 [1]
7777777777777777777777777777777777777777777777777777777777777777 // (8) element 4 [0]
8888888888888888888888888888888888888888888888888888888888888888 // (9) element 4 [1]

I deduced the above by inspecting the transaction details of the below in remix:

pragma solidity ^0.4.24;
contract myContract {
    event Debug(bytes data);
    function myDynamicFunction(uint[2][] myArray) public returns (bytes) {       
        address(this).callcode(bytes4(sha3("myDynamicFunction(uint[2][])")), myArray);
        return msg.data;
    }
    function () {
        emit Debug(msg.data); //reading the event logs shows that the delegatecall is forwarding incorrectly encoded msg.data!
    }
}

How can I edit the assembly so that dynamic arrays are passed to the contract correctly, by the Proxy? Current assembly in the proxy:

assembly {
      let ptr := mload(0x40)
      calldatacopy(ptr, 0, calldatasize)
      let result := delegatecall(gas, _impl, ptr, calldatasize, 0, 0)
      let size := returndatasize
      returndatacopy(ptr, 0, size)

      switch result
      case 0 { revert(ptr, size) }
      default { return(ptr, size) }
    }
spalladino commented 5 years ago

@MichaelConnorOfficial I'm not being able to reproduce this. I've set up the following contract, and calling into the foo function both directly and via a proxy works fine:

contract Foo {
    function foo(uint256[2][] xss) public pure returns (uint256) {
        return xss[0][0] + xss[1][0] + xss[0][1] + xss[1][1];
    }
}

I'm using Solidity 0.4.24. Can you share the full code of the contract you are testing with (the one that throws the error, not the one running the Debug), the error you are getting, and the compiler version? Thanks!

hobbydev71 commented 2 years ago

Is this issue not solved yet?

frangio commented 2 years ago

This is not an issue, the proxy just forwards all msg.data regardless of the type that it encodes.