From e64ca4a9d7ee24b0e92646047a4a2d0097da661c Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Mon, 9 Sep 2024 11:07:53 +0100 Subject: [PATCH 1/8] feat: add onlyEVCAccount modifier --- src/utils/EVCUtil.sol | 35 +++++++++++----- test/invariants/EthereumVaultConnector.t.sol | 14 +++---- test/unit/EVCUtil/EVCUtil.t.sol | 42 ++++++++++++++++---- 3 files changed, 66 insertions(+), 25 deletions(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index cf572951..072d039c 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -49,6 +49,18 @@ abstract contract EVCUtil { _; } + /// @notice Ensures a standard authentication path on the EVC allowing the account owner or any EVC account. + /// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context. + /// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress. + /// @dev This modifier must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw. + /// @dev This modifier must not be used on checkAccountStatus and checkVaultStatus functions. + /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on + /// the EVC. + modifier onlyEVCAccount() virtual { + _onlyEVCAccount(false); + _; + } + /// @notice Ensures a standard authentication path on the EVC. /// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context. /// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress. @@ -59,7 +71,7 @@ abstract contract EVCUtil { /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on /// the EVC. modifier onlyEVCAccountOwner() virtual { - _onlyEVCAccountOwner(); + _onlyEVCAccount(true); _; } @@ -159,12 +171,13 @@ abstract contract EVCUtil { } } - /// @notice Ensures that the function is called only by the EVC account owner + /// @notice Ensures that the function is called only by the EVC account owner or any EVC account /// @dev This function checks if the caller is the EVC and if so, verifies that the execution context is not in a - /// special state (operator authenticated, collateral control in progress, or checks in progress). If the owner was - /// already registered on the EVC, it verifies that the onBehalfOfAccount is the owner. - /// @dev Reverts if the caller is not the EVC or if the execution context is in a special state. - function _onlyEVCAccountOwner() internal view { + /// special state (operator authenticated, collateral control in progress, or checks in progress). If + /// onlyAccountOwner is true and the owner was already registered on the EVC, it verifies that the onBehalfOfAccount + /// is the owner. If onlyAccountOwner is false, it allows any EVC account to call the function. + /// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account + function _onlyEVCAccount(bool onlyAccountOwner) internal view { if (msg.sender == address(evc)) { EC ec = EC.wrap(evc.getRawExecutionContext()); @@ -172,11 +185,13 @@ abstract contract EVCUtil { revert NotAuthorized(); } - address onBehalfOfAccount = ec.getOnBehalfOfAccount(); - address owner = evc.getAccountOwner(onBehalfOfAccount); + if (onlyAccountOwner) { + address onBehalfOfAccount = ec.getOnBehalfOfAccount(); + address owner = evc.getAccountOwner(onBehalfOfAccount); - if (owner != address(0) && owner != onBehalfOfAccount) { - revert NotAuthorized(); + if (owner != address(0) && owner != onBehalfOfAccount) { + revert NotAuthorized(); + } } } } diff --git a/test/invariants/EthereumVaultConnector.t.sol b/test/invariants/EthereumVaultConnector.t.sol index de6426f1..7da064e5 100644 --- a/test/invariants/EthereumVaultConnector.t.sol +++ b/test/invariants/EthereumVaultConnector.t.sol @@ -230,7 +230,9 @@ contract EthereumVaultConnectorHandler is EthereumVaultConnectorScribble, Test { return; } - function batchSimulation(BatchItem[] calldata) + function batchSimulation( + BatchItem[] calldata + ) public payable override @@ -257,13 +259,9 @@ contract EthereumVaultConnectorHandler is EthereumVaultConnectorScribble, Test { accountControllers[account].firstElement = firstElementCache; } - function checkAccountStatusInternal(address account) - internal - view - virtual - override - returns (bool isValid, bytes memory result) - { + function checkAccountStatusInternal( + address account + ) internal view virtual override returns (bool isValid, bytes memory result) { SetStorage storage accountControllersStorage = accountControllers[account]; uint256 numOfControllers = accountControllersStorage.numElements; diff --git a/test/unit/EVCUtil/EVCUtil.t.sol b/test/unit/EVCUtil/EVCUtil.t.sol index 9defd542..bf53b7bf 100644 --- a/test/unit/EVCUtil/EVCUtil.t.sol +++ b/test/unit/EVCUtil/EVCUtil.t.sol @@ -27,12 +27,9 @@ contract EVCClient is EVCUtil { return param; } - function calledThroughEVCPayableBytes(bytes calldata param) - external - payable - callThroughEVC - returns (bytes memory) - { + function calledThroughEVCPayableBytes( + bytes calldata param + ) external payable callThroughEVC returns (bytes memory) { require(msg.sender == address(evc), "Not EVC"); return param; } @@ -41,6 +38,10 @@ contract EVCClient is EVCUtil { // do nothing } + function calledByEVCAccount() external onlyEVCAccount { + // do nothing + } + function calledByEVCAccountOwner() external onlyEVCAccountOwner { // do nothing } @@ -187,17 +188,24 @@ contract EVCUtilTest is Test { evcClient.calledByEVCWithChecksInProgress(); } - function test_calledByEVCAccountOwner(address caller, uint8 id) external { + function test_calledByEVCAccount_calledByEVCAccountOwner(address caller, uint8 id) external { vm.assume(!evc.haveCommonOwner(caller, address(0)) && !evc.haveCommonOwner(caller, address(evc))); vm.assume(id != 0); // msg.sender is not EVC evc.setOnBehalfOfAccount(address(0)); + vm.prank(caller); + evcClient.calledByEVCAccount(); + vm.prank(caller); evcClient.calledByEVCAccountOwner(); // msg.sender is EVC and operator is authenticated evc.setOperatorAuthenticated(true); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.calledByEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); @@ -205,6 +213,10 @@ contract EVCUtilTest is Test { // msg.sender is EVC and control collateral is in progress evc.setOperatorAuthenticated(false); evc.setControlCollateralInProgress(true); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.calledByEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); @@ -212,6 +224,10 @@ contract EVCUtilTest is Test { // msg.sender is EVC and checks are in progress evc.setControlCollateralInProgress(false); evc.setChecksInProgress(true); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.calledByEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); @@ -219,6 +235,10 @@ contract EVCUtilTest is Test { // msg.sender is EVC, the owner is not registered yet evc.setChecksInProgress(false); evc.setOnBehalfOfAccount(caller); + assertEq(evc.getAccountOwner(caller), address(0)); + vm.prank(address(evc)); + evcClient.calledByEVCAccount(); + assertEq(evc.getAccountOwner(caller), address(0)); vm.prank(address(evc)); evcClient.calledByEVCAccountOwner(); @@ -227,6 +247,11 @@ contract EVCUtilTest is Test { evc.setOnBehalfOfAccount(address(0)); vm.prank(caller); evc.call(address(0), caller, 0, ""); + assertEq(evc.getAccountOwner(caller), caller); + evc.setOnBehalfOfAccount(caller); + vm.prank(address(evc)); + evcClient.calledByEVCAccount(); + assertEq(evc.getAccountOwner(caller), caller); evc.setOnBehalfOfAccount(caller); vm.prank(address(evc)); @@ -234,6 +259,9 @@ contract EVCUtilTest is Test { // msg.sender is EVC, the owner is registered but the authenticated account is not the owner evc.setOnBehalfOfAccount(address(uint160(uint160(caller) ^ id))); + vm.prank(address(evc)); + evcClient.calledByEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); From 650497dc201aee82a725f6919a6107b7ecf70d68 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Thu, 3 Oct 2024 14:03:54 +0100 Subject: [PATCH 2/8] chore: improve natspec --- src/utils/EVCUtil.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index 072d039c..65c6d46e 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -49,7 +49,7 @@ abstract contract EVCUtil { _; } - /// @notice Ensures a standard authentication path on the EVC allowing the account owner or any EVC account. + /// @notice Ensures a standard authentication path on the EVC allowing the account owner or any of its EVC account. /// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context. /// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress. /// @dev This modifier must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw. From 25585ae00f32911b62dda393e1c9021ee8f06d9f Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Thu, 3 Oct 2024 14:04:13 +0100 Subject: [PATCH 3/8] fix: typo --- src/utils/EVCUtil.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index 65c6d46e..fe98ba1c 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -49,7 +49,7 @@ abstract contract EVCUtil { _; } - /// @notice Ensures a standard authentication path on the EVC allowing the account owner or any of its EVC account. + /// @notice Ensures a standard authentication path on the EVC allowing the account owner or any of its EVC accounts. /// @dev This modifier checks if the caller is the EVC and if so, verifies the execution context. /// It reverts if the operator is authenticated, control collateral is in progress, or checks are in progress. /// @dev This modifier must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw. From fc8ff4f44c5de644d914a05cced32be267ac4c21 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Fri, 4 Oct 2024 13:36:50 +0100 Subject: [PATCH 4/8] chore: improve natspec --- src/utils/EVCUtil.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index fe98ba1c..63a0f465 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -171,12 +171,12 @@ abstract contract EVCUtil { } } - /// @notice Ensures that the function is called only by the EVC account owner or any EVC account + /// @notice Ensures that the function is called only by the EVC account owner or any of its EVC accounts /// @dev This function checks if the caller is the EVC and if so, verifies that the execution context is not in a /// special state (operator authenticated, collateral control in progress, or checks in progress). If /// onlyAccountOwner is true and the owner was already registered on the EVC, it verifies that the onBehalfOfAccount - /// is the owner. If onlyAccountOwner is false, it allows any EVC account to call the function. - /// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account + /// is the owner. If onlyAccountOwner is false, it allows any EVC account of the owner to call the function. + /// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account of the owner function _onlyEVCAccount(bool onlyAccountOwner) internal view { if (msg.sender == address(evc)) { EC ec = EC.wrap(evc.getRawExecutionContext()); From 9006e67eefc57a296333f02743cb999dfa9724f1 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Fri, 11 Oct 2024 14:09:41 +0200 Subject: [PATCH 5/8] chore: fix foundry version in the CI --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5a9bd1d1..8b829733 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -34,7 +34,7 @@ jobs: submodules: recursive - uses: foundry-rs/foundry-toolchain@v1 with: - version: nightly + version: nightly-c99854277c346fa6de7a8f9837230b36fd85850e - name: Run foundry fmt check run: | From 3058f08727e2c11010ee4dd3a148b28b02783a61 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Fri, 11 Oct 2024 14:18:03 +0200 Subject: [PATCH 6/8] chore: forge fmt --- test/invariants/EthereumVaultConnector.t.sol | 14 ++++++++------ test/unit/EVCUtil/EVCUtil.t.sol | 9 ++++++--- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/test/invariants/EthereumVaultConnector.t.sol b/test/invariants/EthereumVaultConnector.t.sol index 7da064e5..de6426f1 100644 --- a/test/invariants/EthereumVaultConnector.t.sol +++ b/test/invariants/EthereumVaultConnector.t.sol @@ -230,9 +230,7 @@ contract EthereumVaultConnectorHandler is EthereumVaultConnectorScribble, Test { return; } - function batchSimulation( - BatchItem[] calldata - ) + function batchSimulation(BatchItem[] calldata) public payable override @@ -259,9 +257,13 @@ contract EthereumVaultConnectorHandler is EthereumVaultConnectorScribble, Test { accountControllers[account].firstElement = firstElementCache; } - function checkAccountStatusInternal( - address account - ) internal view virtual override returns (bool isValid, bytes memory result) { + function checkAccountStatusInternal(address account) + internal + view + virtual + override + returns (bool isValid, bytes memory result) + { SetStorage storage accountControllersStorage = accountControllers[account]; uint256 numOfControllers = accountControllersStorage.numElements; diff --git a/test/unit/EVCUtil/EVCUtil.t.sol b/test/unit/EVCUtil/EVCUtil.t.sol index bf53b7bf..3139e6ce 100644 --- a/test/unit/EVCUtil/EVCUtil.t.sol +++ b/test/unit/EVCUtil/EVCUtil.t.sol @@ -27,9 +27,12 @@ contract EVCClient is EVCUtil { return param; } - function calledThroughEVCPayableBytes( - bytes calldata param - ) external payable callThroughEVC returns (bytes memory) { + function calledThroughEVCPayableBytes(bytes calldata param) + external + payable + callThroughEVC + returns (bytes memory) + { require(msg.sender == address(evc), "Not EVC"); return param; } From 83c5bb91cdaa28cbd41fb2b24795a8dacefa69ee Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Wed, 16 Oct 2024 10:27:50 +0200 Subject: [PATCH 7/8] feat: add _msgSenderOnlyEVCAccountOwner --- src/utils/EVCUtil.sol | 21 +++++++++++++++---- test/unit/EVCUtil/EVCUtil.t.sol | 37 +++++++++++++++++++++++++++++++-- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index 63a0f465..a06e8c1f 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -57,7 +57,7 @@ abstract contract EVCUtil { /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on /// the EVC. modifier onlyEVCAccount() virtual { - _onlyEVCAccount(false); + _authenticateCallerWithStandardContextState(false); _; } @@ -71,7 +71,7 @@ abstract contract EVCUtil { /// @dev This modifier can be used on access controlled functions to prevent non-standard authentication paths on /// the EVC. modifier onlyEVCAccountOwner() virtual { - _onlyEVCAccount(true); + _authenticateCallerWithStandardContextState(true); _; } @@ -133,6 +133,13 @@ abstract contract EVCUtil { return sender; } + /// @notice Retrieves the message sender, ensuring it's the EVC account owner and that the execution context is in a + /// standard state (not operator authenticated, not control collateral in progress, not checks in progress). + /// @return The address of the message sender. + function _msgSenderOnlyEVCAccountOwner() internal view returns (address) { + return _authenticateCallerWithStandardContextState(true); + } + /// @notice Calls the current external function through the EVC. /// @dev This function is used to route the current call through the EVC if it's not already coming from the EVC. It /// makes the EVC set the execution context and call back this contract with unchanged calldata. msg.sender is used @@ -177,7 +184,8 @@ abstract contract EVCUtil { /// onlyAccountOwner is true and the owner was already registered on the EVC, it verifies that the onBehalfOfAccount /// is the owner. If onlyAccountOwner is false, it allows any EVC account of the owner to call the function. /// @param onlyAccountOwner If true, only allows the account owner; if false, allows any EVC account of the owner - function _onlyEVCAccount(bool onlyAccountOwner) internal view { + /// @return The address of the message sender. + function _authenticateCallerWithStandardContextState(bool onlyAccountOwner) internal view returns (address) { if (msg.sender == address(evc)) { EC ec = EC.wrap(evc.getRawExecutionContext()); @@ -185,14 +193,19 @@ abstract contract EVCUtil { revert NotAuthorized(); } + address onBehalfOfAccount = ec.getOnBehalfOfAccount(); + if (onlyAccountOwner) { - address onBehalfOfAccount = ec.getOnBehalfOfAccount(); address owner = evc.getAccountOwner(onBehalfOfAccount); if (owner != address(0) && owner != onBehalfOfAccount) { revert NotAuthorized(); } } + + return onBehalfOfAccount; } + + return msg.sender; } } diff --git a/test/unit/EVCUtil/EVCUtil.t.sol b/test/unit/EVCUtil/EVCUtil.t.sol index 3139e6ce..cf316751 100644 --- a/test/unit/EVCUtil/EVCUtil.t.sol +++ b/test/unit/EVCUtil/EVCUtil.t.sol @@ -57,6 +57,10 @@ contract EVCClient is EVCUtil { return _msgSenderForBorrow(); } + function msgSenderOnlyEVCAccountOwner() external view returns (address) { + return _msgSenderOnlyEVCAccountOwner(); + } + fallback(bytes calldata) external returns (bytes memory) { return abi.encode(IVault.checkAccountStatus.selector); } @@ -191,7 +195,10 @@ contract EVCUtilTest is Test { evcClient.calledByEVCWithChecksInProgress(); } - function test_calledByEVCAccount_calledByEVCAccountOwner(address caller, uint8 id) external { + function test_calledByEVCAccount_calledByEVCAccountOwner_msgSenderOnlyEVCAccountOwner( + address caller, + uint8 id + ) external { vm.assume(!evc.haveCommonOwner(caller, address(0)) && !evc.haveCommonOwner(caller, address(evc))); vm.assume(id != 0); @@ -203,6 +210,9 @@ contract EVCUtilTest is Test { vm.prank(caller); evcClient.calledByEVCAccountOwner(); + vm.prank(caller); + assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); + // msg.sender is EVC and operator is authenticated evc.setOperatorAuthenticated(true); vm.prank(address(evc)); @@ -213,6 +223,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccountOwner(); + // msg.sender is EVC and control collateral is in progress evc.setOperatorAuthenticated(false); evc.setControlCollateralInProgress(true); @@ -224,6 +238,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccountOwner(); + // msg.sender is EVC and checks are in progress evc.setControlCollateralInProgress(false); evc.setChecksInProgress(true); @@ -235,6 +253,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccountOwner(); + // msg.sender is EVC, the owner is not registered yet evc.setChecksInProgress(false); evc.setOnBehalfOfAccount(caller); @@ -246,6 +268,10 @@ contract EVCUtilTest is Test { vm.prank(address(evc)); evcClient.calledByEVCAccountOwner(); + assertEq(evc.getAccountOwner(caller), address(0)); + vm.prank(address(evc)); + assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); + // msg.sender is EVC, the owner is registered and the authenticated account is the owner evc.setOnBehalfOfAccount(address(0)); vm.prank(caller); @@ -256,10 +282,13 @@ contract EVCUtilTest is Test { evcClient.calledByEVCAccount(); assertEq(evc.getAccountOwner(caller), caller); - evc.setOnBehalfOfAccount(caller); vm.prank(address(evc)); evcClient.calledByEVCAccountOwner(); + assertEq(evc.getAccountOwner(caller), caller); + vm.prank(address(evc)); + assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); + // msg.sender is EVC, the owner is registered but the authenticated account is not the owner evc.setOnBehalfOfAccount(address(uint160(uint160(caller) ^ id))); vm.prank(address(evc)); @@ -268,6 +297,10 @@ contract EVCUtilTest is Test { vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccountOwner(); } function test_msgSender(address caller) external { From 3686aea9ada405b85833febf103c65c622499e27 Mon Sep 17 00:00:00 2001 From: kasperpawlowski Date: Wed, 16 Oct 2024 10:40:22 +0200 Subject: [PATCH 8/8] feat: add _msgSenderOnlyEVCAccount --- src/utils/EVCUtil.sol | 16 ++++++++++++++++ test/unit/EVCUtil/EVCUtil.t.sol | 32 +++++++++++++++++++++++++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/utils/EVCUtil.sol b/src/utils/EVCUtil.sol index a06e8c1f..598b7468 100644 --- a/src/utils/EVCUtil.sol +++ b/src/utils/EVCUtil.sol @@ -133,8 +133,24 @@ abstract contract EVCUtil { return sender; } + /// @notice Retrieves the message sender, ensuring it's any EVC account meaning that the execution context is in a + /// standard state (not operator authenticated, not control collateral in progress, not checks in progress). + /// @dev This function must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw. + /// @dev This function must not be used on checkAccountStatus and checkVaultStatus functions. + /// @dev This function can be used on access controlled functions to prevent non-standard authentication paths on + /// the EVC. + /// @return The address of the message sender. + function _msgSenderOnlyEVCAccount() internal view returns (address) { + return _authenticateCallerWithStandardContextState(false); + } + /// @notice Retrieves the message sender, ensuring it's the EVC account owner and that the execution context is in a /// standard state (not operator authenticated, not control collateral in progress, not checks in progress). + /// @dev It assumes that if the caller is not the EVC, the caller is the account owner. + /// @dev This function must not be used on functions utilized by liquidation flows, i.e. transfer or withdraw. + /// @dev This function must not be used on checkAccountStatus and checkVaultStatus functions. + /// @dev This function can be used on access controlled functions to prevent non-standard authentication paths on + /// the EVC. /// @return The address of the message sender. function _msgSenderOnlyEVCAccountOwner() internal view returns (address) { return _authenticateCallerWithStandardContextState(true); diff --git a/test/unit/EVCUtil/EVCUtil.t.sol b/test/unit/EVCUtil/EVCUtil.t.sol index cf316751..22b22869 100644 --- a/test/unit/EVCUtil/EVCUtil.t.sol +++ b/test/unit/EVCUtil/EVCUtil.t.sol @@ -57,6 +57,10 @@ contract EVCClient is EVCUtil { return _msgSenderForBorrow(); } + function msgSenderOnlyEVCAccount() external view returns (address) { + return _msgSenderOnlyEVCAccount(); + } + function msgSenderOnlyEVCAccountOwner() external view returns (address) { return _msgSenderOnlyEVCAccountOwner(); } @@ -195,7 +199,7 @@ contract EVCUtilTest is Test { evcClient.calledByEVCWithChecksInProgress(); } - function test_calledByEVCAccount_calledByEVCAccountOwner_msgSenderOnlyEVCAccountOwner( + function test_calledByEVCAccount_calledByEVCAccountOwner_msgSenderOnlyEVCAccount_msgSenderOnlyEVCAccountOwner( address caller, uint8 id ) external { @@ -210,6 +214,9 @@ contract EVCUtilTest is Test { vm.prank(caller); evcClient.calledByEVCAccountOwner(); + vm.prank(caller); + assertEq(evcClient.msgSenderOnlyEVCAccount(), caller); + vm.prank(caller); assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); @@ -223,6 +230,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.msgSenderOnlyEVCAccountOwner(); @@ -238,6 +249,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.msgSenderOnlyEVCAccountOwner(); @@ -253,6 +268,10 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); + evcClient.msgSenderOnlyEVCAccount(); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.msgSenderOnlyEVCAccountOwner(); @@ -268,6 +287,10 @@ contract EVCUtilTest is Test { vm.prank(address(evc)); evcClient.calledByEVCAccountOwner(); + assertEq(evc.getAccountOwner(caller), address(0)); + vm.prank(address(evc)); + assertEq(evcClient.msgSenderOnlyEVCAccount(), caller); + assertEq(evc.getAccountOwner(caller), address(0)); vm.prank(address(evc)); assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); @@ -285,6 +308,10 @@ contract EVCUtilTest is Test { vm.prank(address(evc)); evcClient.calledByEVCAccountOwner(); + assertEq(evc.getAccountOwner(caller), caller); + vm.prank(address(evc)); + assertEq(evcClient.msgSenderOnlyEVCAccount(), caller); + assertEq(evc.getAccountOwner(caller), caller); vm.prank(address(evc)); assertEq(evcClient.msgSenderOnlyEVCAccountOwner(), caller); @@ -298,6 +325,9 @@ contract EVCUtilTest is Test { vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.calledByEVCAccountOwner(); + vm.prank(address(evc)); + assertEq(evcClient.msgSenderOnlyEVCAccount(), address(uint160(uint160(caller) ^ id))); + vm.prank(address(evc)); vm.expectRevert(abi.encodeWithSelector(EVCUtil.NotAuthorized.selector)); evcClient.msgSenderOnlyEVCAccountOwner();