Fix secondary index scope isolation and harden chain_plugin#284
Open
Fix secondary index scope isolation and harden chain_plugin#284
Conversation
Secondary index iteration in get_table_rows leaked entries across scopes because sec_key lacked a scope discriminator. The CDT now encodes [scope:8B BE] as a prefix on secondary keys and stores pri_key as [pk:8B] (down from 16B). Chain-side, the unused by_code_table_idx_prikey index is dropped, reducing kv_index_object billing from 152 to 120 bytes per entry. Chain plugin hardening: - collect_next validates sec_key scope prefix and minimum size before pointer arithmetic (prevents size_t underflow and cross-scope next_key leak on deadline timeout) - pri_key size guard (kv_pri_key_size) in both forward and reverse iteration prevents OOB read from malformed kv_index_object - Reverse iteration uses break (not continue) on scope mismatch for consistency with forward path - scope_next_be computation moved inside non-overflow branch New constants: kv_scope_prefix_size, kv_pri_key_size Tests: 7 cross-scope tests (iteration, find, erase, upper_bound, double secondary, scope=0, reverse iteration) Recompiled contracts (sysio.system, sysio.roa) and regenerated snapshot reference data for updated billing.
Recompile all test contracts that use secondary indexes with the updated CDT that encodes [scope:8B][value] in secondary keys. Without this, chain_plugin get_table_rows queries return 0 rows because the scope-prefixed bounds don't match the stored keys. Bump kv_idx_payer_billing RAM allocation from 1.0 to 1.1 SYS to accommodate the test_kv_api WASM size with the 10x set_code multiplier.
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
1 participant
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.
Summary
Bug: Secondary index iteration in
get_table_rowsleaked entries across scopes becausesec_keyhad no scope discriminator.Fix (CDT-side encoding, no intrinsic signature changes):
sec_key: prepend[scope:8B BE]prefixpri_key: shrink from[scope:8B][pk:8B]to[pk:8B]by_code_table_idx_prikeychainbase index (consensus change)kv_index_objectbilling: 152 → 120 bytes per entry (-21%)Chain plugin hardening:
collect_nextvalidates scope prefix and minimum size before pointer arithmetic (preventssize_tunderflow and cross-scopenext_keyleak on deadline timeout)pri_keysize guard (kv_pri_key_size) in both forward and reverse iteration prevents OOB readkv_scope_prefix_size,kv_pri_key_sizeTests: 7 cross-scope tests (iteration, find, erase, upper_bound, double secondary, scope=0, reverse)
Recompiled production contracts and regenerated snapshot reference data
Updated RAM billing docs with EOS mainnet estimates from actual snapshot data
Depends on: Wire-Network/wire-cdt#47 (CDT encoding changes)