Open
Conversation
… from a private slot (#110)
This test demonstrates the bug where ArrayUtils::clearArray uses hardcoded SSTORE instead of CSTORE for small shielded arrays. The test shows that deleting shielded arrays like suint256[3] or saddress[4] incorrectly generates SSTORE instructions in the unrolled loop optimization path. This will be fixed in the next commit.
The original fix in commit 5c75476 addressed GenericStorageItem::setToZero, but ArrayUtils::clearArray had an optimization path that bypassed it. For small fixed-size arrays (storageSize <= 5), clearArray uses an unrolled loop that directly emits store instructions. This code path was using hardcoded SSTORE without checking if the base type is shielded. This meant that operations like `delete suint256[3]` would incorrectly use SSTORE instead of CSTORE, leaking private data to public storage. The fix checks if the array's base type is shielded and uses CSTORE for shielded types, SSTORE for non-shielded types. Affected code path: - ArrayUtils::clearArray lines 568-578 (unrolled loop for small arrays) Other code paths correctly use StorageItem::setToZero which handles shielded types properly after the original fix.
Tests are currently failing since right now we only restrict returning shielded addresses and integers.
4 of the tests were relying on returning shielded types, which now need to be modified to unshield the types before returning them from public functions.
…al fcts This fixes veridise issue 759. Moved the check from DeclarationTypeChecker to TypeChecker because with the new semantic we need to recursively check all child AST nodes (members of structs for example). Keeping the check in DeclarationTypeChecker was throwing some assert errors on some children nodes that haven't been previously been type checked themselves. Having the check done in TypeChecker also feels more natural anyways, since its checking for return types and there was already a function named checkArgumentAndReturnParameter.
Verifies that the timestampms instruction is rejected when targeting EVM versions older than Mercury.
Add TIMESTAMPMS to the same version gate as CLOAD/CSTORE in both EVMVersion::hasOpcode() and AsmAnalysis. This prevents generation of bytecode containing opcode 0x4b for VMs that don't support it.
…y typechecker level
Add EVM version checks in TypeChecker.cpp for timestamp_ms and timestamp_seconds magic block members. Users targeting non-Mercury VMs will now get a clear compile-time error instead of generating bytecode with invalid opcodes that crash at runtime.
The SMT model creates separate symbolic variables for block.timestamp and block.timestamp_seconds even though they resolve to the same opcode. This causes `assert(block.timestamp == block.timestamp_seconds)` to fail spuriously in the SMT checker.
The SMT encoder was creating separate symbolic variables for block.timestamp and block.timestamp_seconds, even though they resolve to the same TIMESTAMP opcode at runtime. This could cause spurious assertion failures when both were used in the same contract. Following the existing pattern for difficulty/prevrandao aliasing: - SMTEncoder.cpp: alias timestamp_seconds → timestamp in both Identifier and MagicType code paths - Predicate.cpp: alias in TxVarsVisitor for counterexample formatting - SymbolicState.cpp: remove independent constraint for timestamp_seconds - SymbolicTypes.cpp: remove timestamp_seconds from transaction member types
…ed types Verify that bytes/string storage operations (push, pop, resize, assign) compile correctly. These operations use byte array helpers that should only emit sload/sstore since there is no shielded dynamic bytes type (sbytesN is a fixed-size value type, not a dynamic byte array).
…opcode selection Four byte array helper functions (resizeDynamicByteArrayFunction, increaseByteArraySizeFunction, byteArrayTransitLongToShortFunction, storageByteArrayPopFunction) chose between sload/cload and sstore/cstore based on _type.category() == ShieldedInteger. Since _type is an ArrayType, its category is always Array, making the shielded branch unreachable. Fix by checking _type.baseType()->isShielded() instead, which correctly inspects the element type. This has no behavioral change today (bytes/string have a non-shielded base type) but prepares these helpers for a future shielded dynamic bytes type.
Add tests verifying the compiler rejects shielded types in: - abi.encode() with suint256, saddress, sbool, structs, and arrays - abi.encodePacked() with shielded types - abi.encodeWithSelector() and abi.encodeWithSignature() - abi.decode() with shielded type targets Update existing tests to expect the new error and cast shielded NONCE to uint96 in AESEncryptAndDecryptWithRng semantic test. These tests expose a vulnerability where shielded values could be serialized to plaintext bytes via ABI encoding, leaking confidential data and allowing recovery via abi.decode.
Add type checks in TypeChecker to prevent shielded values from being serialized through ABI encoding functions, which would leak confidential data as plaintext bytes. Changes: - typeCheckABIDecodeAndRetrieveReturnType(): reject shielded types in abi.decode() target types (error 4851) - typeCheckABIEncodeFunctions(): reject shielded types in abi.encode(), abi.encodePacked(), abi.encodeWithSelector(), and abi.encodeWithSignature() (error 3648) - typeCheckABIEncodeCallFunction(): reject shielded types in abi.encodeCall() arguments (error 3648) Uses containsShieldedType() to catch both direct shielded types and composite types (structs, arrays) containing shielded fields. Checks are guarded with else-if to only trigger when the type would otherwise be valid for encoding, avoiding interference with existing error paths.
…ented) shielded bytes (#176) Replace remaining hardcoded sstore/sload with templated storeOpcode/ loadOpcode in byte array storage helpers, using baseType()->isShielded() to select between cstore/cload and sstore/sload. This sets up dead code to be correct for when sbytes (dynamic shielded bytes) is implemented. Functions fixed: - copyByteArrayToStorageFunction: 5 sstore + 1 sload + read template - decreaseByteArraySizeFunction: 2 sstore - increaseByteArraySizeFunction: 3 sstore - storageByteArrayPopFunction: 2 sstore - storageArrayPushFunction: 5 sstore/sload in byte array path
…lded fields Covers the case where delete on a struct containing both shielded and non-shielded fields incorrectly clears packed non-shielded slots with cstore instead of sstore.
clearStorageStructFunction used containsShieldedType() as a struct-level flag to choose cstore vs sstore for all slot clears. For mixed structs this caused non-shielded packed fields to be cleared with cstore while their getters use sload, reverting under privacy rules. Since all shielded types have storageBytes() == 32, the storageBytes < 32 branch can only ever process non-shielded fields. Hardcode sstore in that path and remove the unused struct-level flag.
Add 4 tests covering the auditor's finding that mixing cstore/sload in inline assembly compiles but reverts at runtime: - shielded_cstore_caller_then_sload: auditor's exact POC (TypeError 5768) - shielded_cstore_then_sload_var_slot: variable-based slot matching (TypeError 5768) - shielded_cstore_then_sload_dynamic_slot_ok: documents known limitation where dynamic slots cannot be caught at compile time - cstore_then_sload_dynamic_slot_should_fail: proves dynamic slot case correctly reverts at runtime The core fix already exists in validateShieldedStorageOps() (ad883e1). These tests confirm coverage of the auditor's specific scenarios and the acknowledged limitation around dynamic slot expressions.
Update all tests involving dynamic shielded arrays to expect .length to return suint256 instead of uint256, and to use cload/cstore instead of sload/sstore for the length slot in inline assembly. Add new semantic and syntax tests for comprehensive coverage of the suint length behavior including type checking, cload/cstore assembly verification, push/pop operations, and mapping interactions.
Dynamic shielded arrays (e.g. suint[], saddress[]) now store their length in confidential storage (cload/cstore) instead of public storage (sload/sstore), and .length returns suint256 instead of uint256. This prevents leaking collection size metadata through storage access patterns. Fixed-length shielded arrays (e.g. suint[5]) are unaffected — their .length remains uint256 (compile-time constant). All conditionals use containsShieldedType() rather than checking the base type directly, so future sbytes support will automatically inherit shielded length behavior. Changes: - Types.cpp: .length member returns suint256 for dynamic shielded arrays - ArrayUtils.cpp: 6 locations switch sload/sstore to cload/cstore for length in legacy codegen (copyArrayToStorage, clearDynamicArray, resizeDynamicArray, incrementDynamicArraySize, popStorageArrayElement, retrieveLength) - YulUtilFunctions.cpp: 5 locations switch sload/sstore to cload/cstore for length in Yul codegen (arrayLengthFunction, resizeArrayFunction, storageArrayPopFunction, storageArrayPushFunction, storageArrayPushZeroFunction)
…ion contexts Update test expectations to include warnings (9660, 9661, 9662, 9663, 1457) when shielded type conversions from literals or constant expressions appear in assignments, function arguments, return statements, and other non-declaration contexts. Add new test files for struct, array, nested array, fixedbytes, and constant expression literal-to-shielded warnings.
Move literal-to-shielded warning check from endVisit(VariableDeclarationStatement) into typeCheckTypeConversionAndRetrieveReturnType so it fires for all explicit type conversions — assignments, function arguments, return statements, and array pushes — not just variable declarations. Extract the warning logic into a reusable checkLiteralToShielded helper that checks annotation().type-based cases (RationalNumber, Enum) before the Literal AST node cast, allowing constant expressions like 0 ** 1E1233 to also trigger warnings. Add checkMsgValueToShielded helper for msg.value-to-shielded warnings.
Run semantic tests in 4 configurations: 1. No optimizer, no IR (baseline) 2. With optimizer, no IR (baseline) 3. No optimizer, --via-ir (tests IR codegen) 4. With optimizer, --via-ir (tests optimized IR)
- shielded_array_storage_pop_zero_length.sol: Change EVMVersion from >=petersburg to >=mercury (suint[] requires CLOAD/CSTORE opcodes) - all_possible_user_defined_value_types_with_operators.sol: Add compileViaYul:false with explanatory comment (test exceeds EIP-170's 24KB contract size limit when compiled with via-IR without optimizer)
Add 4 regression tests with compileViaYul:also to ensure via-IR specific bugs are caught while maintaining non-IR test coverage: 1. shielded_struct_delete_mixed_viair.sol Tests that struct delete uses correct opcodes (sstore for uint16/uint8, cstore for sbytes1) when clearing mixed shielded/non-shielded fields. Prevents regression of clearStorageStructFunction bug. 2. sbytes_dynamic_packed_storage_viair.sol Tests sbytes1 array operations (push, index read, index write) use numBytes() not storageBytes() for shift/mask in packed storage. Prevents regression of packed storage bugs. 3. sbytes_abi_encoding_viair.sol Tests storage-to-memory copy uses cload not sload for sbytes arrays. Prevents regression of abiEncodingFunctionCompactStorageArray bug. 4. timestamp_magic_viair.sol Tests block.timestamp_seconds and block.timestamp_ms generate correct IR (timestamp() and timestampms() respectively). Prevents regression of IR timestamp member bug. All tests use compileViaYul:also to run in both IR and non-IR modes, ensuring comprehensive coverage and regression prevention.
Fixes all remaining issues blocking via-IR compilation for shielded types. Test results: 1641/1642 pass without optimizer (99.94%), 1642/1642 with optimizer (100%). Single failure is unrelated contract size limit. Issue 1: clearStorageStructFunction used wrong opcode for mixed structs - Problem: Mixed structs (uint16 + sbytes) used cstore for ALL fields - Fix: All shielded types have storageBytes()==32, so <32 branch is non-shielded only. Hardcode sstore with defensive assertion. - File: libsolidity/codegen/YulUtilFunctions.cpp:1915-1931 Issue 2: IR timestamp magic members called non-existent functions - Problem: Generated IR called timestamp_seconds() and timestamp_ms() - Fix: Map timestamp_seconds to timestamp(), timestamp_ms to timestampms() - File: libsolidity/codegen/ir/IRGeneratorForStatements.cpp:1979-1984 Issue 3: ABI routing bug for sbytes storage arrays - Problem: sbytes has storageBytes()==32 but IS a byte array - Fix: Check isByteArrayOrString() before storageBytes() check - File: libsolidity/codegen/ABIFunctions.cpp:328-335 Issue 4: Storage-to-memory copy used sload instead of cload - Problem: abiEncodingFunctionCompactStorageArray used sload for sbytes - Fix: Add loadOpcode template parameter checking isShielded() - File: libsolidity/codegen/ABIFunctions.cpp:727 Issue 5-7: Packed storage operations used storageBytes() not numBytes() - Problem: sbytes1 has storageBytes()=32 but occupies 1 byte in arrays - Fix: Use numBytes() for shift/mask calculations in packed storage - Files: libsolidity/codegen/YulUtilFunctions.cpp:3180,3235,2978 Issue 8: Memory byte array write rejected sbytes1 - Problem: Assertion expected only bytes1 for memory byte array elements - Fix: Accept both bytes1 and sbytes1 - File: libsolidity/codegen/ir/IRGeneratorForStatements.cpp:3233 Additional: Add --unsafe-via-ir flag to CLI and Standard JSON - Allows experimental use of via-IR pipeline with shielded types - Files: solc/CommandLineParser.cpp, libsolidity/interface/StandardCompiler.cpp
Add comprehensive test coverage for shielded type validation across different EVM versions. Tests verify that shielded types are rejected on pre-Mercury EVM versions with error 9978. State storage variables: - shielded_storage_evm_version_paris.sol: Single suint256 on Paris - shielded_storage_evm_version_cancun.sol: Multiple shielded types on Cancun - shielded_storage_struct_evm_version.sol: Struct containing shielded type - shielded_storage_sbytes_evm_version.sol: Shielded byte array Other variable contexts: - shielded_local_variable_evm_version.sol: Local function variable - shielded_immutable_evm_version.sol: Immutable variable (error 7491) - shielded_constant_evm_version.sol: Constant variable (error 7491) Positive test: - shielded_storage_evm_version_mercury.sol: Confirms Mercury support All tests run with both legacy and via-IR pipelines (compileViaYul: true). Note: Immutable and constant tests expect error 7491 (existing validation) rather than 9978, as shielded types cannot be immutable or constant.
Prevent emission of CLOAD/CSTORE opcodes on pre-Mercury EVM versions when using shielded types (suint, sbool, saddress, sbytes) in ANY context. ## Problem Compiling with --evm-version paris (or any pre-Mercury version) incorrectly emitted cload/cstore opcodes for shielded types, causing "invalid opcode" runtime errors on pre-Mercury VMs. ## Solution: Two-Layer Defense ### Layer 1: Type Checker (libsolidity/analysis/TypeChecker.cpp) Add comprehensive early validation in visit(VariableDeclaration) to reject ALL uses of shielded types on pre-Mercury versions: **Checked contexts:** - State storage variables (regular, non-constant, non-immutable) - Local storage references/pointers - Local value-type variables (stack/memory) - Immutable variables (value types only) - Constant variables **Safety considerations:** - For immutables: only check if value type (non-value types already error at line 532) - For constants: always safe (must be value types) - For state variables: always safe (storage location guaranteed) - For local storage: safe (storage location) - For local value types: safe (no nested mapping issues) Error 9978 provides actionable message guiding users to --evm-version mercury. ### Layer 2: Codegen Assertions **Legacy Pipeline (libsolidity/codegen/LValue.cpp)** Add 6 defensive assertions to catch any cases that slip through: - GenericStorageItem::retrieveValue() - line 237 - GenericStorageItem::storeValue() - line 310 - GenericStorageItem::setToZero() - line 521 - StorageByteArrayElement::retrieveValue() - line 569 - StorageByteArrayElement::storeValue() - line 584 - StorageByteArrayElement::setToZero() - line 614 **Via-IR Pipeline (libsolidity/codegen/YulUtilFunctions.cpp)** Add assertions in key functions: - readFromStorageValueType() - line 2864 - resizeDynamicByteArrayFunction() - line 1448 - storageArrayPopFunction() - line 1648 - storageArrayPushZeroFunction() - line 1799 ## Coverage - All variable contexts (state, local, immutable, constant) - Arrays/structs containing shielded types (containsShieldedType check) - Shielded byte arrays (StorageByteArrayElement handlers) - Both legacy and via-IR compilation pipelines - All shielded type categories (suint, sbool, saddress, sbytes)
Verify that EVMVersion() and EVMVersion::current() produce the same default version, catching mismatches between currentVersion and the m_version member initializer.
Set currentVersion to Mercury and initialize m_version from currentVersion, ensuring EVMVersion() and EVMVersion::current() always produce the same default version.
Add tests for LoadResolver and EqualStoreEliminator to verify that cstore/cload confidential storage operations are optimized by the data-flow analysis passes, and that cross-domain independence between sstore/sload and cstore/cload is maintained. Includes a test verifying that cstore invalidates sload knowledge for the same key.
Add ConfidentialStorage as a third StoreLoadLocation in the DataFlowAnalyzer, enabling LoadResolver and EqualStoreEliminator to optimize cload/cstore the same way they handle sload/sstore. cstore/cload operate on an independent storage domain from sstore/sload, so they are tracked in a separate environment map. When cstore(k, v) is encountered, regular storage knowledge for key k is invalidated because a subsequent sload would HALT at runtime (the slot is now private). The reverse invalidation is not needed: sstore on a private slot always HALTs, so sstore can never affect confidential storage.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.