Skip to content

Cap CompactArcStore state count by StateId and validate header counts before narrowing. - #349

Merged
copybara-service[bot] merged 1 commit into
mainfrom
copybara/995337607
Oct 7, 2026
Merged

copybara-service[bot] merged 1 commit into
mainfrom
copybara/995337607

Conversation

@copybara-service

Copy link
Copy Markdown

Cap CompactArcStore state count by StateId and validate header counts before narrowing.

In CompactArcStore<Element, Unsigned> (lib/compact-fst.h):

  1. CompactArcCompactor::NumStates() returns StateId (int32_t for StdArc,
    max 2^31 - 1), whereas kMaxStates is 1ULL << 52 (CompactArcStore is
    not templated on Arc, but its constructors and Read() are templated on
    ArcCompactor).
  2. In CompactArcStore::Read, the 64-bit signed hdr.NumStates() and
    hdr.NumArcs() values were assigned to unsigned size_t (data->nstates_
    and data->narcs_) before checking bounds, making data->nstates_ < 0 and
    data->narcs_ < 0 dead unsigned comparisons.

Fix by:

  • Making kMaxStates and kMaxArcs public on CompactArcStore.
  • Capping the maximum allowed state count in CompactArcStore's constructors
    and Read() to std::min<uint64_t>(kMaxStates, std::numeric_limits<StateId>::max()).
  • Validating hdr.NumStates(), hdr.Start(), and hdr.NumArcs() as int64_t
    in CompactArcStore::Read before assigning them to data->nstates_,
    data->start_, and data->narcs_.
  • Expanding CompactLimitsTest in fst_limits_test.cc to test
    std::numeric_limits<StdArc::StateId>::max() + 1 and negative counts.

@copybara-service
copybara-service Bot force-pushed the copybara/995337607 branch 2 times, most recently from 4e1966e to b17752e Compare October 7, 2026 22:11
…unts before narrowing.

In `CompactArcStore<Element, Unsigned>` (`lib/compact-fst.h`):
1. `CompactArcCompactor::NumStates()` returns `StateId` (`int32_t` for `StdArc`,
   max `2^31 - 1`), whereas `kMaxStates` is `1ULL << 52` (`CompactArcStore` is
   not templated on `Arc`, but its constructors and `Read()` are templated on
   `ArcCompactor`).
2. In `CompactArcStore::Read`, the 64-bit signed `hdr.NumStates()` and
   `hdr.NumArcs()` values were assigned to unsigned `size_t` (`data->nstates_`
   and `data->narcs_`) before checking bounds, making `data->nstates_ < 0` and
   `data->narcs_ < 0` dead unsigned comparisons.

Fix by:
- Making `kMaxStates` and `kMaxArcs` public on `CompactArcStore`.
- Capping the maximum allowed state count in `CompactArcStore`'s constructors
  and `Read()` to `std::min<uint64_t>(kMaxStates, std::numeric_limits<StateId>::max())`.
- Validating `hdr.NumStates()`, `hdr.Start()`, and `hdr.NumArcs()` as `int64_t`
  in `CompactArcStore::Read` before assigning them to `data->nstates_`,
  `data->start_`, and `data->narcs_`.
- Expanding `CompactLimitsTest` in `fst_limits_test.cc` to test
  `std::numeric_limits<StdArc::StateId>::max() + 1` and negative counts.

PiperOrigin-RevId: 995395588
@copybara-service
copybara-service Bot merged commit 45c78f8 into main Oct 7, 2026
1 check passed
@copybara-service
copybara-service Bot deleted the copybara/995337607 branch October 7, 2026 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant