Skip to content

Commit 60d3082

Browse files
authored
fix(host-contracts): upgrade FHEVMExecutor to v0.2.0 (#2144)
* fix: reject scalar div/rem values that truncate to zero * fix: make executor hotfix upgrade-safe * test: cover scalar div rem truncation across widths * chore: bump executor minor version for hotfix upgrades * chore(host-contracts): refresh executor rust bindings
1 parent e5ad77f commit 60d3082

File tree

5 files changed

+279
-17
lines changed

5 files changed

+279
-17
lines changed

host-contracts/contracts/FHEVMExecutor.sol

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
122122
uint256 private constant MAJOR_VERSION = 0;
123123

124124
/// @notice Minor version of the contract.
125-
uint256 private constant MINOR_VERSION = 1;
125+
uint256 private constant MINOR_VERSION = 2;
126126

127127
/// @notice Patch version of the contract.
128128
uint256 private constant PATCH_VERSION = 0;
@@ -138,7 +138,7 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
138138

139139
/// Constant used for making sure the version number used in the `reinitializer` modifier is
140140
/// identical between `initializeFromEmptyProxy` and the `reinitializeVX` method
141-
uint64 private constant REINITIALIZER_VERSION = 2;
141+
uint64 private constant REINITIALIZER_VERSION = 3;
142142

143143
/// keccak256(abi.encode(uint256(keccak256("fhevm.storage.FHEVMExecutor")) - 1)) & ~bytes32(uint256(0xff))
144144
bytes32 private constant FHEVMExecutorStorageLocation =
@@ -161,7 +161,7 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
161161
*/
162162
/// @custom:oz-upgrades-unsafe-allow missing-initializer-call
163163
/// @custom:oz-upgrades-validate-as-initializer
164-
// function reinitializeV2() public virtual reinitializer(REINITIALIZER_VERSION) {}
164+
function reinitializeV2() public virtual reinitializer(REINITIALIZER_VERSION) {}
165165

166166
/**
167167
* @notice Computes FHEAdd operation.
@@ -229,13 +229,13 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
229229
*/
230230
function fheDiv(bytes32 lhs, bytes32 rhs, bytes1 scalarByte) public virtual returns (bytes32 result) {
231231
if (scalarByte != 0x01) revert IsNotScalar(); /// @dev we know scalarByte is either 0x01 or 0x00 because we check it is boolean inside _binaryOp
232-
if (rhs == 0) revert DivisionByZero();
233232
uint256 supportedTypes = (1 << uint8(FheType.Uint8)) +
234233
(1 << uint8(FheType.Uint16)) +
235234
(1 << uint8(FheType.Uint32)) +
236235
(1 << uint8(FheType.Uint64)) +
237236
(1 << uint8(FheType.Uint128));
238237
FheType lhsType = _verifyAndReturnType(lhs, supportedTypes);
238+
if (_isScalarZeroForType(rhs, lhsType)) revert DivisionByZero();
239239
result = _binaryOp(Operators.fheDiv, lhs, rhs, scalarByte, lhsType);
240240
hcuLimit.checkHCUForFheDiv(lhsType, scalarByte, lhs, rhs, result);
241241
emit FheDiv(msg.sender, lhs, rhs, scalarByte, result);
@@ -250,13 +250,13 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
250250
*/
251251
function fheRem(bytes32 lhs, bytes32 rhs, bytes1 scalarByte) public virtual returns (bytes32 result) {
252252
if (scalarByte != 0x01) revert IsNotScalar(); /// @dev we know scalarByte is either 0x01 or 0x00 because we check it is boolean inside _binaryOp
253-
if (rhs == 0) revert DivisionByZero();
254253
uint256 supportedTypes = (1 << uint8(FheType.Uint8)) +
255254
(1 << uint8(FheType.Uint16)) +
256255
(1 << uint8(FheType.Uint32)) +
257256
(1 << uint8(FheType.Uint64)) +
258257
(1 << uint8(FheType.Uint128));
259258
FheType lhsType = _verifyAndReturnType(lhs, supportedTypes);
259+
if (_isScalarZeroForType(rhs, lhsType)) revert DivisionByZero();
260260
result = _binaryOp(Operators.fheRem, lhs, rhs, scalarByte, lhsType);
261261
hcuLimit.checkHCUForFheRem(lhsType, scalarByte, lhs, rhs, result);
262262
emit FheRem(msg.sender, lhs, rhs, scalarByte, result);
@@ -810,6 +810,18 @@ contract FHEVMExecutor is UUPSUpgradeableEmptyProxy, FHEEvents, ACLOwnable {
810810
if ((1 << uint8(typeCt)) & supportedTypes == 0) revert UnsupportedType();
811811
}
812812

813+
function _isScalarZeroForType(bytes32 scalar, FheType scalarType) internal pure virtual returns (bool) {
814+
uint256 scalarUint = uint256(scalar);
815+
816+
if (scalarType == FheType.Uint8) return uint8(scalarUint) == 0;
817+
if (scalarType == FheType.Uint16) return uint16(scalarUint) == 0;
818+
if (scalarType == FheType.Uint32) return uint32(scalarUint) == 0;
819+
if (scalarType == FheType.Uint64) return uint64(scalarUint) == 0;
820+
if (scalarType == FheType.Uint128) return uint128(scalarUint) == 0;
821+
822+
revert UnsupportedType();
823+
}
824+
813825
function _unaryOp(Operators op, bytes32 ct) internal virtual returns (bytes32 result) {
814826
if (!acl.isAllowed(ct, msg.sender)) revert ACLNotAllowed(ct, msg.sender);
815827
result = keccak256(abi.encodePacked(op, ct, acl, block.chainid));

host-contracts/rust_bindings/src/fhevm_executor.rs

Lines changed: 195 additions & 5 deletions
Large diffs are not rendered by default.

host-contracts/test/fhevm-foundry/TestHostContractsDeployerTestUtils.t.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ contract TestHostContractsDeployerTestUtils is HostContractsDeployerTestUtils {
3535

3636
assertEq(address(fhevmExecutorProxy), fhevmExecutorAdd, "FHEVMExecutor proxy address mismatch");
3737
assertNotEq(fhevmExecutorImplementation, address(0), "Implementation not deployed");
38-
assertEq(fhevmExecutorProxy.getVersion(), "FHEVMExecutor v0.1.0", "Version mismatch");
38+
assertEq(fhevmExecutorProxy.getVersion(), "FHEVMExecutor v0.2.0", "Version mismatch");
3939
assertEq(
4040
_readImplementationSlot(fhevmExecutorAdd),
4141
fhevmExecutorImplementation,

host-contracts/test/fhevmExecutor/fhevmExecutor.t.sol

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ contract FHEVMExecutorTest is SupportedTypesConstants, Test {
368368
assertEq(fhevmExecutor.getInputVerifierAddress(), inputVerifierAdd);
369369
assertEq(fhevmExecutor.getACLAddress(), aclAdd);
370370
assertEq(fhevmExecutor.getHCULimitAddress(), hcuLimitAdd);
371-
assertEq(fhevmExecutor.getVersion(), string(abi.encodePacked("FHEVMExecutor v0.1.0")));
371+
assertEq(fhevmExecutor.getVersion(), string(abi.encodePacked("FHEVMExecutor v0.2.0")));
372372
}
373373

374374
/// @dev This function exists for the test below to call it externally.
@@ -491,10 +491,9 @@ contract FHEVMExecutorTest is SupportedTypesConstants, Test {
491491
bytes1 scalarByte = bytes1(0x01);
492492

493493
bytes32 lhs = _generateMockHandle(FheType(fheType));
494-
bytes32 rhs = _generateMockHandle(FheType(fheType));
494+
bytes32 rhs = bytes32(uint256(2));
495495

496496
_approveHandleInACL(lhs, sender);
497-
_approveHandleInACL(rhs, sender);
498497

499498
bytes32 expectedResult = _computeExpectedResultBinaryOp(
500499
FHEVMExecutor.Operators.fheDiv,
@@ -520,10 +519,9 @@ contract FHEVMExecutorTest is SupportedTypesConstants, Test {
520519
bytes1 scalarByte = bytes1(0x01);
521520

522521
bytes32 lhs = _generateMockHandle(FheType(fheType));
523-
bytes32 rhs = _generateMockHandle(FheType(fheType));
522+
bytes32 rhs = bytes32(uint256(2));
524523

525524
_approveHandleInACL(lhs, sender);
526-
_approveHandleInACL(rhs, sender);
527525

528526
bytes32 expectedResult = _computeExpectedResultBinaryOp(
529527
FHEVMExecutor.Operators.fheRem,
@@ -1745,6 +1743,68 @@ contract FHEVMExecutorTest is SupportedTypesConstants, Test {
17451743
fhevmExecutor.fheRem(lhs, rhs, 0x01);
17461744
}
17471745

1746+
function _expectScalarDivRevertsWhenScalarTruncatesToZero(FheType fheType, uint256 rhsValue) internal {
1747+
bytes32 lhs = _generateMockHandle(fheType);
1748+
bytes32 rhs = bytes32(rhsValue);
1749+
address account = address(123);
1750+
_approveHandleInACL(lhs, account);
1751+
1752+
vm.expectRevert(FHEVMExecutor.DivisionByZero.selector);
1753+
vm.prank(account);
1754+
fhevmExecutor.fheDiv(lhs, rhs, 0x01);
1755+
}
1756+
1757+
function _expectScalarRemRevertsWhenScalarTruncatesToZero(FheType fheType, uint256 rhsValue) internal {
1758+
bytes32 lhs = _generateMockHandle(fheType);
1759+
bytes32 rhs = bytes32(rhsValue);
1760+
address account = address(123);
1761+
_approveHandleInACL(lhs, account);
1762+
1763+
vm.expectRevert(FHEVMExecutor.DivisionByZero.selector);
1764+
vm.prank(account);
1765+
fhevmExecutor.fheRem(lhs, rhs, 0x01);
1766+
}
1767+
1768+
function test_RevertsIfFheDivUint8ScalarTruncatesToZero() public {
1769+
_expectScalarDivRevertsWhenScalarTruncatesToZero(FheType.Uint8, 1 << 8);
1770+
}
1771+
1772+
function test_RevertsIfFheDivUint16ScalarTruncatesToZero() public {
1773+
_expectScalarDivRevertsWhenScalarTruncatesToZero(FheType.Uint16, 1 << 16);
1774+
}
1775+
1776+
function test_RevertsIfFheDivUint32ScalarTruncatesToZero() public {
1777+
_expectScalarDivRevertsWhenScalarTruncatesToZero(FheType.Uint32, 1 << 32);
1778+
}
1779+
1780+
function test_RevertsIfFheDivUint64ScalarTruncatesToZero() public {
1781+
_expectScalarDivRevertsWhenScalarTruncatesToZero(FheType.Uint64, 1 << 64);
1782+
}
1783+
1784+
function test_RevertsIfFheDivUint128ScalarTruncatesToZero() public {
1785+
_expectScalarDivRevertsWhenScalarTruncatesToZero(FheType.Uint128, 1 << 128);
1786+
}
1787+
1788+
function test_RevertsIfFheRemUint8ScalarTruncatesToZero() public {
1789+
_expectScalarRemRevertsWhenScalarTruncatesToZero(FheType.Uint8, 1 << 8);
1790+
}
1791+
1792+
function test_RevertsIfFheRemUint16ScalarTruncatesToZero() public {
1793+
_expectScalarRemRevertsWhenScalarTruncatesToZero(FheType.Uint16, 1 << 16);
1794+
}
1795+
1796+
function test_RevertsIfFheRemUint32ScalarTruncatesToZero() public {
1797+
_expectScalarRemRevertsWhenScalarTruncatesToZero(FheType.Uint32, 1 << 32);
1798+
}
1799+
1800+
function test_RevertsIfFheRemUint64ScalarTruncatesToZero() public {
1801+
_expectScalarRemRevertsWhenScalarTruncatesToZero(FheType.Uint64, 1 << 64);
1802+
}
1803+
1804+
function test_RevertsIfFheRemUint128ScalarTruncatesToZero() public {
1805+
_expectScalarRemRevertsWhenScalarTruncatesToZero(FheType.Uint128, 1 << 128);
1806+
}
1807+
17481808
function test_RevertsIfFheDivRHSIsNotScalar() public {
17491809
bytes32 lhs = _generateMockHandle(FheType.Uint16);
17501810
bytes32 rhs = _generateMockHandle(FheType.Uint16);

host-contracts/test/upgrades/upgrades.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe('Upgrades', function () {
6969
call: { fn: 'initializeFromEmptyProxy' },
7070
});
7171
await executor.waitForDeployment();
72-
expect(await executor.getVersion()).to.equal('FHEVMExecutor v0.1.0');
72+
expect(await executor.getVersion()).to.equal('FHEVMExecutor v0.2.0');
7373
const executor2 = await upgrades.upgradeProxy(executor, executorFactoryUpgraded);
7474
await executor2.waitForDeployment();
7575
expect(await executor2.getVersion()).to.equal('FHEVMExecutor v0.4.0');

0 commit comments

Comments
 (0)