diff --git a/contracts/LSP6KeyManager/LSP6Errors.sol b/contracts/LSP6KeyManager/LSP6Errors.sol index d3101ea0a..745231633 100644 --- a/contracts/LSP6KeyManager/LSP6Errors.sol +++ b/contracts/LSP6KeyManager/LSP6Errors.sol @@ -165,3 +165,8 @@ error RelayCallBeforeStartTime(); * @dev reverts when the period to execute the relay call has expired */ error RelayCallExpired(); + +/** + * @dev reverts when the address of the Key Manager is being set as extensions for lsp20 functions + */ +error KeyManagerCannotBeSetAsExtensionForLSP20Functions(); diff --git a/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol b/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol index cb6d4d546..868b0b244 100644 --- a/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol +++ b/contracts/LSP6KeyManager/LSP6Modules/LSP6SetDataModule.sol @@ -1,6 +1,11 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.5; +// interfaces +import { + ILSP20CallVerifier as ILSP20 +} from "../../LSP20CallVerification/ILSP20CallVerifier.sol"; + // modules import {ERC725Y} from "@erc725/smart-contracts/contracts/ERC725Y.sol"; import { @@ -43,7 +48,8 @@ import { InvalidEncodedAllowedERC725YDataKeys, NoERC725YDataKeysAllowed, NotAllowedERC725YDataKey, - NotAuthorised + NotAuthorised, + KeyManagerCannotBeSetAsExtensionForLSP20Functions } from "../LSP6Errors.sol"; abstract contract LSP6SetDataModule { @@ -299,6 +305,18 @@ abstract contract LSP6SetDataModule { // LSP17Extension: } else if (bytes12(inputDataKey) == _LSP17_EXTENSION_PREFIX) { + // reverts when the address of the Key Manager is being set as extensions for lsp20 functions + bytes4 selector = bytes4(inputDataKey << 96); + + if ( + (selector == ILSP20.lsp20VerifyCall.selector || + selector == ILSP20.lsp20VerifyCallResult.selector) + ) { + if (address(bytes20(inputDataValue)) == address(this)) { + revert KeyManagerCannotBeSetAsExtensionForLSP20Functions(); + } + } + // same as above. If controller has both permissions, do not read the `target` storage // to save gas by avoiding an extra external `view` call. if ( diff --git a/tests/LSP6KeyManager/Admin/PermissionChangeAddExtensions.test.ts b/tests/LSP6KeyManager/Admin/PermissionChangeAddExtensions.test.ts index f4832c92e..edf7f091d 100644 --- a/tests/LSP6KeyManager/Admin/PermissionChangeAddExtensions.test.ts +++ b/tests/LSP6KeyManager/Admin/PermissionChangeAddExtensions.test.ts @@ -193,6 +193,86 @@ export const shouldBehaveLikePermissionChangeOrAddExtensions = ( const result = await context.universalProfile.getData(payloadParam.dataKey); expect(result).to.equal(payloadParam.dataValue); }); + + it('should revert when adding the KeyManager address as extensions for lsp20VerifyCall function', async () => { + const lsp20VerifyCallSelector = + context.keyManager.interface.getSighash('lsp20VerifyCall'); + + const payloadParam = { + dataKey: + ERC725YDataKeys.LSP17.LSP17ExtensionPrefix + + lsp20VerifyCallSelector.slice(2) + + '00000000000000000000000000000000', // zero padded, + dataValue: context.keyManager.address, + }; + + const payload = context.universalProfile.interface.encodeFunctionData('setData', [ + payloadParam.dataKey, + payloadParam.dataValue, + ]); + + await expect( + context.keyManager.connect(context.owner).execute(payload), + ).to.be.revertedWithCustomError( + context.keyManager, + 'KeyManagerCannotBeSetAsExtensionForLSP20Functions', + ); + }); + + it('should revert when adding the KeyManager address as extensions for lsp20VerifyCallResult function', async () => { + const lsp20VerifyCallResultSelector = + context.keyManager.interface.getSighash('lsp20VerifyCallResult'); + + const payloadParam = { + dataKey: + ERC725YDataKeys.LSP17.LSP17ExtensionPrefix + + lsp20VerifyCallResultSelector.slice(2) + + '00000000000000000000000000000000', // zero padded, + dataValue: context.keyManager.address, + }; + + const payload = context.universalProfile.interface.encodeFunctionData('setData', [ + payloadParam.dataKey, + payloadParam.dataValue, + ]); + + await expect( + context.keyManager.connect(context.owner).execute(payload), + ).to.be.revertedWithCustomError( + context.keyManager, + 'KeyManagerCannotBeSetAsExtensionForLSP20Functions', + ); + }); + + it('should pass when adding the random address as extensions for lsp20VerifyCall and lsp20VerifyCallResult functions', async () => { + const lsp20VerifyCallSelector = + context.keyManager.interface.getSighash('lsp20VerifyCall'); + + const lsp20VerifyCallResultSelector = + context.keyManager.interface.getSighash('lsp20VerifyCallResult'); + + const payloadParam = { + dataKeys: [ + ERC725YDataKeys.LSP17.LSP17ExtensionPrefix + + lsp20VerifyCallSelector.slice(2) + + '00000000000000000000000000000000', + ERC725YDataKeys.LSP17.LSP17ExtensionPrefix + + lsp20VerifyCallResultSelector.slice(2) + + '00000000000000000000000000000000', + ], // zero padded, + dataValues: [context.accounts[2].address, context.accounts[3].address], + }; + + const payload = context.universalProfile.interface.encodeFunctionData('setDataBatch', [ + payloadParam.dataKeys, + payloadParam.dataValues, + ]); + + await context.keyManager.connect(context.owner).execute(payload); + + const result = await context.universalProfile.getDataBatch(payloadParam.dataKeys); + expect(result).to.deep.equal(payloadParam.dataValues); + }); }); describe('when caller is an address with ADD/CHANGE Extensions permission', () => {