Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: PermissionlessGenericHandler: unpack depositor address with custom length #161

Merged
merged 7 commits into from
Apr 7, 2023
Merged

chore: PermissionlessGenericHandler: unpack depositor address with custom length #161

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c1b7037
Unpack depositor address with custom length
viatrix Mar 22, 2023
36598d1
Add comment
viatrix Mar 22, 2023
ed9a92f
Add helpers for PermissionlessGenericHandler
viatrix Apr 5, 2023
5f86a9d
Update helpers for PermissionlessGenericHandler
viatrix Apr 5, 2023
8eef864
Update comments
viatrix Apr 7, 2023
e29ca1c
Update comment
viatrix Apr 7, 2023
8d3a003
Update comments
viatrix Apr 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions contracts/handlers/PermissionlessGenericHandler.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ contract PermissionlessGenericHandler is IHandler {
executionData: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor) - END
*/
function deposit(bytes32 resourceID, address depositor, bytes calldata data) external view returns (bytes memory) {
require(data.length > 81, "Incorrect data length");
require(data.length >= 76, "Incorrect data length"); // 32 + 2 + 1 + 1 + 20 + 20

uint16 lenExecuteFuncSignature;
uint8 lenExecuteContractAddress;
Expand All @@ -67,7 +67,7 @@ contract PermissionlessGenericHandler is IHandler {
lenExecuteFuncSignature = uint16(bytes2(data[32:34]));
lenExecuteContractAddress = uint8(bytes1(data[34 + lenExecuteFuncSignature:35 + lenExecuteFuncSignature]));
lenExecutionDataDepositor = uint8(bytes1(data[35 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress]));
executionDataDepositor = abi.decode(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor], (address));
executionDataDepositor = address(uint160(bytes20(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor])));

require(depositor == executionDataDepositor, 'incorrect depositor in deposit data');
}
Expand All @@ -82,22 +82,27 @@ contract PermissionlessGenericHandler is IHandler {
len(executeContractAddress): uint8 bytes 34 + len(executeFuncSignature) - 35 + len(executeFuncSignature)
executeContractAddress bytes bytes 35 + len(executeFuncSignature) - 35 + len(executeFuncSignature) + len(executeContractAddress)
len(executionDataDepositor): uint8 bytes 35 + len(executeFuncSignature) + len(executeContractAddress) - 36 + len(executeFuncSignature) + len(executeContractAddress)
executionDataDepositorWithData: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) - END
executionDataDepositor: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) - 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor)
executionData: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor) - END
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
executionDataDepositor: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) - 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor)
executionData: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor) - END
executionDataDepositor: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) - 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor)
executionData: bytes bytes 36 + len(executeFuncSignature) + len(executeContractAddress) + len(executionDataDepositor) - END

*/
function executeProposal(bytes32 resourceID, bytes calldata data) external onlyBridge {
uint16 lenExecuteFuncSignature;
bytes4 executeFuncSignature;
uint8 lenExecuteContractAddress;
address executeContractAddress;
bytes memory executionDataDepositorWithData;
uint8 lenExecutionDataDepositor;
address executionDataDepositor;
bytes memory executionData;

lenExecuteFuncSignature = uint16(bytes2(data[32:34]));
executeFuncSignature = bytes4(data[34:34 + lenExecuteFuncSignature]);
lenExecuteContractAddress = uint8(bytes1(data[34 + lenExecuteFuncSignature:35 + lenExecuteFuncSignature]));
executeContractAddress = address(uint160(bytes20(data[35 + lenExecuteFuncSignature:35 + lenExecuteFuncSignature + lenExecuteContractAddress])));
executionDataDepositorWithData = bytes(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress:]);
lenExecutionDataDepositor = uint8(bytes1(data[35 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress]));
executionDataDepositor = address(uint160(bytes20(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress:36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor])));
executionData = bytes(data[36 + lenExecuteFuncSignature + lenExecuteContractAddress + lenExecutionDataDepositor:]);

bytes memory callData = abi.encodePacked(executeFuncSignature, executionDataDepositorWithData);
bytes memory callData = abi.encodePacked(executeFuncSignature, abi.encode(executionDataDepositor), executionData);
lastperson marked this conversation as resolved.
Show resolved Hide resolved
executeContractAddress.call(callData);
}
}
3 changes: 2 additions & 1 deletion test/handlers/generic/permissionlessDeposit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ contract("PermissionlessGenericHandler - [deposit]", async (accounts) => {
});

it("deposit data should be of required length", async () => {
const invalidDepositData = "0x" + "02a3d".repeat(31);
// Min length is 76 bytes
const invalidDepositData = "0x" + "aa".repeat(75);

await TruffleAssert.reverts(
BridgeInstance.deposit(
Expand Down
4 changes: 2 additions & 2 deletions test/helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,8 @@ const createPermissionlessGenericDepositData = (
executeFunctionSignature.substr(2) + // bytes
toHex(executeContractAddress.substr(2).length / 2, 1).substr(2) + // uint8
executeContractAddress.substr(2) + // bytes
toHex(32, 1).substr(2) + // uint8
toHex(depositor, 32).substr(2) + // bytes32
toHex(depositor.substr(2).length / 2, 1).substr(2) + // uint8
depositor.substr(2) + // bytes
executionData.substr(2)
) // bytes
.toLowerCase();
Expand Down