release: v1.2.3-alpha2 — community security and stability fixes - #3
release: v1.2.3-alpha2 — community security and stability fixes#3igorls wants to merge 13 commits into
Conversation
A failing onblock is deterministic and benign: the block still applies, the schedule/state change is simply dropped, and every node reproduces the same failure (e.g. the system contract proposing a producer schedule that the native set_proposed_producers intrinsic rejects). During snapshot catch-up / replay the handler dumped the full onblock trace at warn for every affected block -- observed as 38,923 consecutive warnings over one Telos testnet window. Gate it on the same ">5 min behind head" heuristic update_peer_keys() uses: warn only at/near head, downgrade to dlog while catching up (the full trace is still available by raising the controller logger to debug). Pure logging- severity change -- no consensus, state, or control-flow impact, and the interrupt_exception path is still handled and rethrown before the gate.
…nds, and shared_blob memcpy - Issue 8: fix SHA3 big-endian out-of-bounds write and initialize loop index - Issue 10: add bounds check in message_buffer::advance_read_ptr preventing underflow - Issue 11: add null guard and zero-length handling in to_base58 and EncodeBase58 - Issue 12: guard shared_blob size before calling ds.write in history_pack_big_bytes - Issue 4 helper: add fc::bounded_datastream adapter enforcing max read/write byte bounds - Add unit and adversarial stress test suites for bounded_datastream, base58, message_buffer, and state_history
…inal boundary, and finalizer weight overflow
- Issue 1: hoist multiplier n to size_t in BLS weighted-sum and pairing intrinsics preventing 32-bit modulo wrapping and cross-runtime consensus divergence
- Issue 2: validate pending QC vote bitset format and weights before dual-finalizer vote comparison; add missing bounds assertions
- Issue 5: correct weak_final aggregation boundary comparison to strict '>' allowing strong QC formation at boundary weight
- Issue 7 (chain): record declared_auths_satisfied on transaction_metadata when check_authorization succeeds
- Issue 9: initialize max_weak_sum_before_weak_final() accumulator with uint64_t{0} avoiding signed integer overflow
- Add comprehensive regression and adversarial test suites in unittests
…vely bill verified auths, and bound SHiP queue - Issue 3: only mark block progress for blocks already held; default p2p-disable-block-nack to true for producing nodes - Issue 4: bound P2P frame parsers using fc::bounded_datastream to prevent stream desync and consuming bytes from pipelined frames; add advance_to_frame_end() - Issue 6: bound SHiP queued_status_requests to prevent unbounded queue growth and memory exhaustion - Issue 7 (producer): gate subjective failure billing and account failure tracking on declared_auths_satisfied to prevent DoS against victim accounts - Issue 13: validate host and port syntax in connect() prior to inserting into supplied_peers - Add unit, plugin, and cluster integration tests
Bump VERSION_SUFFIX alpha1->alpha2 so VERSION_FULL = 1.2.3-alpha2 (drives CPACK_PACKAGE_VERSION and the nodeos version string). Second testing alpha of the eosrio/spring 1.2.x maintenance line, incorporating all 13 verified community fixes for consensus divergence, P2P desync, queue memory exhaustion, and undefined behaviors.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d9d1166e54
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| using appbase::app; | ||
|
|
||
| FC_DECLARE_DERIVED_EXCEPTION( status_request_queue_limit_exceeded, chain::plugin_exception, | ||
| 3240001, "State history status request queue limit exceeded" ); |
There was a problem hiding this comment.
When the status-request queue overflows, this exception reports code 3240001, which is already assigned to snapshot_validation_exception in libraries/chain/include/eosio/chain/exceptions.hpp. This is the only duplicate declared exception code in the repository, so clients or monitoring that classify failures by exception::code() will misidentify a SHiP queue-limit disconnect as a snapshot-validation failure; use an unused plugin-exception code instead.
Useful? React with 👍 / 👎.
| if( p > _max_bytes ) return false; | ||
| if( p < _total ) { | ||
| if constexpr ( requires { ds.seekp(p); } ) { | ||
| if( ds.seekp( p ) ) { |
There was a problem hiding this comment.
When a bounded_datastream is created over an underlying stream whose cursor is already nonzero, a backward seekp(p) forwards p as an absolute underlying position rather than a position relative to where the bounded wrapper began. For example, wrapping a stream at byte 100 and then calling seekp(0) moves the underlying cursor to byte 0, allowing subsequent reads to access data outside the intended bounded region while _total and remaining() still report a valid position. Record the initial underlying offset and seek to that offset plus p, or reject backward seeks when that cannot be done safely.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟡 Changes recommended
Two verified issues (SHiP queue extract leaving moved-from state and insufficient peer endpoint validation) should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR bumps Spring to v1.2.3-alpha2 and lands a broad set of security/stability hardening fixes across consensus/finality, P2P networking, state history, producer subjective billing, and core libfc utilities, with extensive regression and adversarial test coverage.
Changes:
- Harden consensus/finality verification and aggregation logic (QC validation ordering, bounds assertions, off-by-one fix, weight accumulation safety).
- Harden P2P framing and connection behavior (bounded message parsing, frame alignment, watchdog progress rules, peer endpoint validation, producer defaults).
- Fix core utility UB/safety issues (
sha3,base58,message_buffer, state history packing) and add targeted unit/adversarial tests.
File summaries
| File | Description |
|---|---|
| unittests/test_m2_adversarial.cpp | Adds adversarial tests covering multiple reported issues (BLS wrap, QC validation, weak-final boundary, weight accumulation, auth satisfaction). |
| unittests/subjective_billing_tests.cpp | Adds unit tests validating failure billing is gated on satisfied authorizations. |
| unittests/state_history_tests.cpp | Adds tests for shared_blob packing safety and SHiP status queue bounding behavior. |
| unittests/finality_misc_tests.cpp | Adds regression tests for weak-final boundary and large-weight accumulation. |
| unittests/bls_primitives_tests.cpp | Adds tests for BLS length-wrap overflow cases. |
| unittests/block_state_tests.cpp | Adds tests ensuring pending QC bitset validation happens before indexing. |
| unittests/auth_tests.cpp | Adds tests for transaction_metadata authorization satisfaction tracking/reset behavior. |
| tests/subjective_billing_test.py | Makes subjective decay assertion polling-based rather than fixed sleep. |
| plugins/state_history_plugin/include/eosio/state_history_plugin/session.hpp | Introduces bounded status-request queue and exception; updates session to enforce queue cap. |
| plugins/producer_plugin/producer_plugin.cpp | Gates subjective failure billing and account failure tracking on satisfied authorizations. |
| plugins/net_plugin/tests/test_net_plugin.cpp | Adds net plugin tests for block-notice progress, bounded parsing, and peer validation. |
| plugins/net_plugin/tests/CMakeLists.txt | Includes new test_net_plugin.cpp in the test target. |
| plugins/net_plugin/net_plugin.cpp | Uses fc::bounded_datastream for bounded parsing; fixes watchdog progress logic; adjusts nack default; validates peer addresses before retention. |
| plugins/net_plugin/include/eosio/net_plugin/net_utils.hpp | Adds block notice classification/progress helpers; exposes endpoint parsing helper used by net/ tests. |
| libraries/state_history/include/eosio/state_history/serialization.hpp | Avoids write(nullptr, 0) for empty shared_blob serialization. |
| libraries/libfc/test/test_m1_adversarial.cpp | Adds adversarial tests for bounded datastream, message buffer bounds, base58, and sha3. |
| libraries/libfc/test/test_base58.cpp | Adds focused base58 safety/roundtrip/invalid-input tests. |
| libraries/libfc/test/network/test_message_buffer.cpp | Adds tests for advance_read_ptr bounds behavior. |
| libraries/libfc/test/io/test_bounded_datastream.cpp | Adds comprehensive bounded datastream tests (read/write/seek/pack/unpack). |
| libraries/libfc/test/CMakeLists.txt | Registers new libfc unit test sources. |
| libraries/libfc/src/crypto/sha3.cpp | Fixes endian macro logic, initializes index, and corrects loop bound (prevents OOB/UB). |
| libraries/libfc/src/crypto/base58.cpp | Avoids UB on empty vector and adds null-pointer guard for to_base58. |
| libraries/libfc/include/fc/network/message_buffer.hpp | Adds bounds check to advance_read_ptr to prevent underflow/corruption. |
| libraries/libfc/include/fc/io/datastream.hpp | Adds fc::bounded_datastream wrapper to enforce max-bytes bounds. |
| libraries/libfc/include/fc/crypto/base58.hpp | Adds default yield parameter to base58 APIs for easier/safe calling. |
| libraries/chain/webassembly/crypto.cpp | Fixes 32-bit length wrap in BLS intrinsics by hoisting n to size_t. |
| libraries/chain/qc.cpp | Reorders pending QC validation before dual-vote indexing; adds bounds assertions; fixes weak-final boundary (>). |
| libraries/chain/include/eosio/chain/transaction_metadata.hpp | Adds declared_auths_satisfied and accessors for satisfied-authorization tracking. |
| libraries/chain/include/eosio/chain/finalizer_policy.hpp | Prevents overflow by using uint64_t{0} accumulator seed. |
| libraries/chain/controller.cpp | Resets/sets declared_auths_satisfied around authorization checks; downgrades noisy onblock warning during replay/sync. |
| docs/RELEASE_NOTES_v1.2.3-alpha2.md | Adds release notes documenting the alpha2 remediation set and verification claims. |
| CMakeLists.txt | Updates version suffix from alpha1 to alpha2. |
| .gitignore | Ignores checksum and test/agent artifact directories. |
Review details
- Files reviewed: 32/33 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| string connections_manager::connect( const string& host, const string& p2p_address ) { | ||
| if (auto [h, port, type] = net_utils::split_host_port_type(host); h.empty()) { | ||
| return "invalid peer address"; | ||
| } |
There was a problem hiding this comment.
Addressed in commit e5053a3: Added net_utils::is_valid_port() checking for numeric digits and range 1..65535, enforced in split_host_port_type() as well as connect() and resolve_and_connect(). Non-numeric ports (such as peer:abc:blk) and out-of-range ports are now strictly rejected with 'invalid peer address'. Added test coverage in plugins/net_plugin/tests/test_net_plugin.cpp.
| std::deque<bool> extract() { | ||
| return std::move(_queue); | ||
| } |
There was a problem hiding this comment.
Addressed in commit e5053a3: status_request_queue::extract() now uses _queue.swap(extracted), deterministically leaving _queue empty with size 0 post-extract. Added regression tests in unittests/state_history_tests.cpp.
…nds and queue extraction - Enforce numeric port range (1..65535) in split_host_port_type and connect/resolve_and_connect - Swap _queue with an empty deque in status_request_queue::extract to ensure deterministic reset post-extract - Add regression tests covering non-numeric ports and queue extract swap behavior
Synchronize LICENSE with upstream AntelopeIO/spring's transition to the MIT License (commit e6a99f6 on 2025-11-12). Update release notes to clarify that upstream adopted MIT prior to this maintenance release.
|
@copilot please review |
Reviewed |
Address review findings on the 1.2.3-alpha2 branch: - CMakeLists.txt: the AntelopeIO#578 guard accepts LLVM 7-11 or 14+, but the FATAL_ERROR still told users to use "an LLVM version 7 through 11". On Ubuntu 24.04/26.04, where llvm-12/13 may be the system default, that misdirected operators toward a downgrade instead of the llvm-18-dev path the new Dockerfiles use. - libraries/libfc/src/exception.cpp: restore the try/catch scaffolding around exception::to_string(), matching to_detail_string() just above it. Callers in net_plugin and cleos invoke it from inside catch(fc::exception&) handlers, where a throw out of format_string would replace the in-flight exception and lose the original diagnostic. FC_CHECK_DEADLINE is intentionally not restored: the loop breaks after the first log entry, so there is nothing left to bound. Claude-Session: https://claude.ai/code/session_017dStsbqQfAGrhfqYQtGHmd
Scoping document for eosrio/spring AntelopeIO#578: bypass ORC entirely rather than migrating to ORCv2, the phased plan (P1-P7), and the differential-correctness/determinism gate that must pass before merge. Companion to the LLVM 14+ build support on this branch. Claude-Session: https://claude.ai/code/session_017dStsbqQfAGrhfqYQtGHmd
Keeps local cluster test output out of the image build context. Claude-Session: https://claude.ai/code/session_017dStsbqQfAGrhfqYQtGHmd
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23f0e3ab5b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if( ds.read( d, s ) ) { | ||
| _total += s; | ||
| return true; |
There was a problem hiding this comment.
When this wrapper is instantiated over the existing datastream<Streambuf>, ds.read(d, s) returns the byte count from sgetn, not a boolean. A short positive read therefore enters this branch, increments _total by the full requested size, and reports success even though the tail of d was not populated; truncated stream input can consequently be accepted and unpacked using uninitialized bytes. Require the underlying result to equal s (or otherwise normalize the read contract) before advancing _total.
Useful? React with 👍 / 👎.
Spring v1.2.3-alpha2 — eosrio/spring maintenance line
eosrio/spring), continuingAntelopeIO/springafter upstream stalled. Built on v1.2.2 + OC-on-LLVM-18 (v1.2.3-alpha1) — with a hard rule of zero compatibility breaks (no consensus/protocol-rule, ABI/serialization/wire, or RPC/SHiP response-shape changes). The v2.0 line is out of scope.Highlights: Comprehensive Community Security & Stability Remediation
This release incorporates an exhaustive audit, patch implementation, and multi-layered verification of 13 community vulnerability and bug reports inherited across Spring-derived nodes (reported by Wire Network in comparative review against
origin/main).Key remediations include:
crypto.cpp) that previously allowed attacker-crafted transactions to trigger out-of-bounds reads, diverging across WASM runtimes (eos-vm-ocvseos-vm/eos-vm-jit).fc::bounded_datastreamto strictly bound message parsers to declared message lengths, ensuring under-length or malicious P2P frames cannot consume bytes from subsequent pipelined messages or desynchronize network streams.weak_finalQC aggregation boundary calculations.message_buffer::advance_read_ptr, guarded null pointer arithmetic into_base58, and eliminatedmemcpy(dst, nullptr, 0)in state history serialization.Categorized Changes
Consensus / chain runtime (Savanna)
nintosize_tprior to multiplication acrossbls_g1_weighted_sum,bls_g2_weighted_sum, andbls_pairing, eliminating integer truncation and cross-runtime execution divergence.qc_t::verify_basicsoverify_vote_formatandverify_weightsrun beforeverify_dual_finalizers_votes; add missingother_vote_indexbounds assertions invote_same_at.weak_finalboundary off by one in QC aggregation (Issue 5): Use strict>comparison understate_t::weak_achievedinqc.cpp, permitting strong QC formation at exact threshold weight.std::accumulatewithuint64_t{0}infinalizer_policy.hppto prevent signed 32-bit integer overflow when finalizer weights exceeddeclared_auths_satisfiedontransaction_metadataupon successfulcheck_authorization()incontroller.cpp.P2P / networking
fc::bounded_datastream, route all net message unpacking through bounded streams, and addconnection::advance_to_frame_end()to skip unconsumed declared bytes without desynchronizing the stream.latest_blk_timeonly when receiving notices for blocks already held in local state; defaultp2p-disable-block-nacktotruewhen a block producer is configured.supplied_peersand non-numeric port rejection (Issue 13): Validate endpoint syntax and enforce numeric port range (1..65535) vianet_utils::is_valid_portandsplit_host_port_typeprior to insertion intosupplied_peersinconnections_manager::connect().Plugins & State History (SHiP & Producer)
queued_status_requestsinsession.hppto 100 entries with deterministic swap extraction to prevent memory exhaustion, and cleanly terminate sessions that exceed the limit withstatus_request_queue_limit_exceeded._account_fails.add) and failure CPU billing ontrx->satisfied_authorizations(), preventing unauthenticated attackers from throttling arbitrary victim accounts.Core utilities & correctness (
libfc,state_history)sha3big-endian OOB write and uninitialized index (Issue 8): Initialize loop indexi = 0, bound word conversion bynumber_of_words = 25, and fix#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__endianness macro checks insha3.cpp.message_buffer::advance_read_ptrbounds check (Issue 10): Add bounds verification throwingfc::out_of_range_exceptionprior to adjusting read pointers, preventing 32-bit unsigned underflow and heap memory corruption.to_base58null pointer safety (Issue 11): Return an empty string for zero-length buffers, assert non-null data pointer on positive length, and guardEncodeBase58overloads against empty containers (data() == nullptr).history_pack_big_bytes(shared_blob)nullmemcpy(Issue 12): Add size guardif (b.size()) ds.write(b.data(), b.size());matching thebytesoverload, eliminating undefined behavior under UBSan.Licensing
AntelopeIO/springtransitioned the project from Business Source License 1.1 (BSL 1.1) to the MIT License in commite6a99f68on November 12, 2025 (prior to the eosrio maintenance releases). Because upstream applied the change tomainwhilerelease/1.2was frozen, the 1.2.x maintenance branch had inadvertently retained the older BSL 1.1 text. This release synchronizesLICENSEwith upstream's official MIT License text.Controller / stability
onblockREJECTING trace out of warn during sync/replay (0b9aca87c).Credits
Special thanks to Wire Network (
Wire-Network/wire-sysio, credit Kevin Heifner) for auditing the shared upstream code, documenting reproducible runtime measurements, and publishing reference fixes, as well as the original community reporters and Spring maintainers.Verification & Validation Evidence
ninja -C build) with zero errors or warnings.test_fcpassed 159/159 test cases and 5,012,360 assertions with 0 failures (includingtest_bounded_datastream,test_base58,test_message_buffer, andtest_m1_adversarial).test_net_pluginpassed 20/20 test cases with 0 failures.unit_testpassed 99/99 test cases acrossbls_primitives_tests,block_state_tests,finality_misc_tests,subjective_billing_tests,state_history_tests,auth_tests, andm2_adversarial_tests.eos-vm-oc,eos-vm(interpreter), andeos-vm-jit.subjective_billing_testpassed in 132.55s.Artifacts & Verification
antelope-spring_1.2.3-alpha2_amd64.deb— Hermetic reproducible build for Ubuntu 20.04 / 22.04 / 24.04 / 26.04 with modern LLVM 18 OC.antelope-spring-1.2.3-alpha2-x86_64.tar.zst— Portable tarball.SHA256SUMS.txt— Cryptographic checksums.Verify binary version string:
nodeos --full-version # Expected: v1.2.3-alpha2-<hash>What to test / known limitations
nodeoswith--eos-vm-oc-enable=alland observe peer block exchange stability and synchronization under notice mode.p2p-disable-block-nack=false) maintain reliable synchronization and that heartbeat watchdog recovery fires promptly if missing blocks are announced.--disable-subjective-p2p-billing=false), verify that spoofed transactions naming valid third-party accounts do not throttle legitimate victim transactions.Full Diff: v1.2.3-alpha1...release/1.2.3-alpha2