diff --git a/contracts/src/spaces/entitlements/rule/RuleEntitlementV2.sol b/contracts/src/spaces/entitlements/rule/RuleEntitlementV2.sol index 483d29097..438ff5872 100644 --- a/contracts/src/spaces/entitlements/rule/RuleEntitlementV2.sol +++ b/contracts/src/spaces/entitlements/rule/RuleEntitlementV2.sol @@ -117,7 +117,11 @@ contract RuleEntitlementV2 is ) external onlySpace { _removeRuleDataV1(roleId); - if (entitlementData.length == 0) return; + // We should never allow the setting of empty rule datas because it can cause the xchain + // architecture to be invoked when by default a user is not entitled. + if (entitlementData.length == 0) { + revert Entitlement__InvalidValue(); + } // equivalent: abi.decode(entitlementData, (RuleDataV2)) RuleDataV2 calldata data; @@ -127,7 +131,11 @@ contract RuleEntitlementV2 is data := add(entitlementData.offset, calldataload(entitlementData.offset)) } - if (data.operations.length == 0) return; + // We should never allow the setting of empty rule datas because it can cause the xchain + // architecture to be invoked when by default a user is not entitled. + if (data.operations.length == 0) { + revert Entitlement__InvalidValue(); + } // Cache lengths of operations arrays to reduce state access cost uint256 operationsLength = data.operations.length; diff --git a/contracts/test/crosschain/RuleEntitlementV2.t.sol b/contracts/test/crosschain/RuleEntitlementV2.t.sol index 83e8dbace..bd1bb2f43 100644 --- a/contracts/test/crosschain/RuleEntitlementV2.t.sol +++ b/contracts/test/crosschain/RuleEntitlementV2.t.sol @@ -75,6 +75,27 @@ contract RuleEntitlementV2Test is RuleEntitlementTest { assertEq(vm.getMappingLength(entitlement, bytes32(ENTITLEMENTS_SLOT)), 0); } + function test_setRuleEntitlement_revertOnEmptyRuleData() public { + test_upgradeToRuleV2(); + + vm.expectRevert(Entitlement__InvalidValue.selector); + + vm.prank(space); + + ruleEntitlementV2.setEntitlement(roleId, ""); + } + + function test_setRuleEntitlement_revertOnZeroLengthRuleData() public { + test_upgradeToRuleV2(); + + vm.expectRevert(Entitlement__InvalidValue.selector); + + vm.prank(space); + + RuleDataV2 memory ruleDataV2; + ruleEntitlementV2.setEntitlement(roleId, abi.encode(ruleDataV2)); + } + function test_removeRuleEntitlement() external override { test_setRuleEntitlement();