diff --git a/constants.ts b/constants.ts index f8e0f7271..613937e7b 100644 --- a/constants.ts +++ b/constants.ts @@ -60,10 +60,10 @@ export const ERC1271_VALUES = { */ export const LSP20_MAGIC_VALUES = { VERIFY_CALL: { - // bytes3(keccak256("lsp20VerifyCall(address,uint256,bytes)")) + "0x01" - WITH_POST_VERIFICATION: '0x1a238001', - // bytes3(keccak256("lsp20VerifyCall(address,uint256,bytes)")) + "0x00" + // bytes3(keccak256("lsp20VerifyCall(address,address,uint256,bytes)")) + "0x00" NO_POST_VERIFICATION: '0x1a238000', + // bytes3(keccak256("lsp20VerifyCall(address,address,uint256,bytes)")) + "0x01" + WITH_POST_VERIFICATION: '0x1a238001', }, // bytes4(keccak256("lsp20VerifyCallResult(bytes32,bytes)")) VERIFY_CALL_RESULT: '0xd3fc45d3', diff --git a/contracts/LSP20CallVerification/ILSP20CallVerifier.sol b/contracts/LSP20CallVerification/ILSP20CallVerifier.sol index 90365761d..4cfb870ee 100644 --- a/contracts/LSP20CallVerification/ILSP20CallVerifier.sol +++ b/contracts/LSP20CallVerification/ILSP20CallVerifier.sol @@ -12,7 +12,7 @@ interface ILSP20CallVerifier { * the function is allowed, concatened with a byte that determines if the lsp20VerifyCallResult function should * be called after the original function call. The byte that invoke the lsp20VerifyCallResult function is strictly `0x01`. * - * @param callee The address of the contract that implements the LSP20 interface + * @param callee The address of the contract that implements the `LSP20CallVerifier` interface * @param caller The address who called the function on the msg.sender * @param value The value sent by the caller to the function called on the msg.sender * @param receivedCalldata The receivedCalldata sent by the caller to the msg.sender diff --git a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol index 68544c9b5..51d483caa 100644 --- a/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol +++ b/contracts/LSP6KeyManager/LSP6KeyManagerCore.sol @@ -98,9 +98,9 @@ abstract contract LSP6KeyManagerCore is // Variables, methods and modifier used for ReentrancyGuard are taken from the link below and modified accordingly. // https://github.com/OpenZeppelin/openzeppelin-contracts/blob/release-v4.8/contracts/security/ReentrancyGuard.sol - mapping(address => uint8) internal _reentrancyStatus; - uint8 internal constant _NOT_ENTERED = 1; - uint8 internal constant _ENTERED = 2; + mapping(address => uint256) internal _reentrancyStatus; + uint256 internal constant _NOT_ENTERED = 1; + uint256 internal constant _ENTERED = 2; /** * @inheritdoc ILSP6 @@ -333,7 +333,7 @@ abstract contract LSP6KeyManagerCore is // If target is invoking the verification, emit the event and change the reentrancy guard if (msg.sender == targetContract) { - uint8 reentrancyStatus = _nonReentrantBefore( + uint256 reentrancyStatus = _nonReentrantBefore( targetContract, isSetData, caller @@ -351,7 +351,7 @@ abstract contract LSP6KeyManagerCore is /// @dev If a different address is invoking the verification, /// do not change the state or emit the event to allow read-only verification else { - uint8 reentrancyStatus = _reentrancyStatus[targetContract]; + uint256 reentrancyStatus = _reentrancyStatus[targetContract]; if (reentrancyStatus == _ENTERED) { _requirePermissions( @@ -399,7 +399,7 @@ abstract contract LSP6KeyManagerCore is address targetContract = _target; - uint8 reentrancyStatus = _nonReentrantBefore( + uint256 reentrancyStatus = _nonReentrantBefore( targetContract, isSetData, msg.sender @@ -476,7 +476,7 @@ abstract contract LSP6KeyManagerCore is bool isSetData = bytes4(payload) == IERC725Y.setData.selector || bytes4(payload) == IERC725Y.setDataBatch.selector; - uint8 reentrancyStatus = _nonReentrantBefore( + uint256 reentrancyStatus = _nonReentrantBefore( targetContract, isSetData, signer @@ -536,6 +536,13 @@ abstract contract LSP6KeyManagerCore is bytes32 permissions = ERC725Y(targetContract).getPermissionsFor(from); if (permissions == bytes32(0)) revert NoPermissionsSet(from); + if (isRelayedCall) { + LSP6ExecuteRelayCallModule._verifyExecuteRelayCallPermission( + from, + permissions + ); + } + bytes4 erc725Function = bytes4(payload); // ERC725Y.setData(bytes32,bytes) @@ -595,13 +602,6 @@ abstract contract LSP6KeyManagerCore is } else { revert InvalidERC725Function(erc725Function); } - - if (isRelayedCall) { - LSP6ExecuteRelayCallModule._verifyExecuteRelayCallPermission( - from, - permissions - ); - } } /** @@ -622,7 +622,7 @@ abstract contract LSP6KeyManagerCore is address targetContract, bool isSetData, address from - ) internal virtual returns (uint8 reentrancyStatus) { + ) internal virtual returns (uint256 reentrancyStatus) { reentrancyStatus = _reentrancyStatus[targetContract]; if (reentrancyStatus == _ENTERED) { // CHECK the caller has REENTRANCY permission diff --git a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md index 3a34b63e6..f757928c4 100644 --- a/docs/contracts/LSP6KeyManager/LSP6KeyManager.md +++ b/docs/contracts/LSP6KeyManager/LSP6KeyManager.md @@ -1204,7 +1204,7 @@ function _nonReentrantBefore( address targetContract, bool isSetData, address from -) internal nonpayable returns (uint8 reentrancyStatus); +) internal nonpayable returns (uint256 reentrancyStatus); ``` Update the status from `_NON_ENTERED` to `_ENTERED` and checks if diff --git a/tests/Reentrancy/LSP20/reentrancyHelpers.ts b/tests/Reentrancy/LSP20/reentrancyHelpers.ts index 63f67332e..bdef32d0c 100644 --- a/tests/Reentrancy/LSP20/reentrancyHelpers.ts +++ b/tests/Reentrancy/LSP20/reentrancyHelpers.ts @@ -261,7 +261,7 @@ export const addUniversalReceiverDelegateTestCases = { NotAuthorised: [ { permissionsText: 'NO Permissions', - permissions: '0x', + permissions: PERMISSIONS.EXECUTE_RELAY_CALL, missingPermission: 'REENTRANCY', }, { @@ -271,12 +271,15 @@ export const addUniversalReceiverDelegateTestCases = { }, { permissionsText: 'REENTRANCY', - permissions: PERMISSIONS.REENTRANCY, + permissions: combinePermissions(PERMISSIONS.EXECUTE_RELAY_CALL, PERMISSIONS.REENTRANCY), missingPermission: 'ADDUNIVERSALRECEIVERDELEGATE', }, { permissionsText: 'ADDUNIVERSALRECEIVERDELEGATE', - permissions: PERMISSIONS.ADDUNIVERSALRECEIVERDELEGATE, + permissions: combinePermissions( + PERMISSIONS.EXECUTE_RELAY_CALL, + PERMISSIONS.ADDUNIVERSALRECEIVERDELEGATE, + ), missingPermission: 'REENTRANCY', }, ], @@ -295,7 +298,7 @@ export const changeUniversalReceiverDelegateTestCases = { NotAuthorised: [ { permissionsText: 'NO Permissions', - permissions: '0x', + permissions: PERMISSIONS.EXECUTE_RELAY_CALL, missingPermission: 'REENTRANCY', }, { @@ -305,12 +308,15 @@ export const changeUniversalReceiverDelegateTestCases = { }, { permissionsText: 'REENTRANCY', - permissions: PERMISSIONS.REENTRANCY, + permissions: combinePermissions(PERMISSIONS.EXECUTE_RELAY_CALL, PERMISSIONS.REENTRANCY), missingPermission: 'CHANGEUNIVERSALRECEIVERDELEGATE', }, { permissionsText: 'CHANGEUNIVERSALRECEIVERDELEGATE', - permissions: PERMISSIONS.CHANGEUNIVERSALRECEIVERDELEGATE, + permissions: combinePermissions( + PERMISSIONS.EXECUTE_RELAY_CALL, + PERMISSIONS.CHANGEUNIVERSALRECEIVERDELEGATE, + ), missingPermission: 'REENTRANCY', }, ],