Skip to content

Cap ConstFstImpl::kMaxStates by StateId and validate header counts before narrowing. - #348

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

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

Conversation

@copybara-service

Copy link
Copy Markdown

Cap ConstFstImpl::kMaxStates by StateId and validate header counts before narrowing.

In ConstFstImpl<Arc, Unsigned> (lib/const-fst.h):

  1. nstates_ and start_ have type StateId (int32_t for StdArc, max
    2^31 - 1), whereas kMaxStates was 1ULL << 52.
  2. In ConstFstImpl::Read, the 64-bit hdr.NumStates(), hdr.Start(), and
    hdr.NumArcs() values were assigned to impl->nstates_ (StateId),
    impl->start_ (StateId), and impl->narcs_ (size_t) before checking
    bounds. As a result, a 64-bit hdr.NumStates() or hdr.Start() such as
    (1LL << 32) + 1 or 1LL << 32 silently truncated to 1 or 0 in 32-bit
    StateId, bypassing the range checks (impl->nstates_ > kMaxStates was
    tautologically false for 32-bit StateId), and impl->narcs_ < 0 was an
    unsigned comparison.

Fix by:

  • Making kMaxStates and kMaxArcs public on ConstFstImpl and capping
    kMaxStates at std::min<uint64_t>(0x10000000000000ull, std::numeric_limits<StateId>::max()).
  • Counting states in uint64_t in ConstFstImpl(const Fst<Arc>&) and checking
    nstates > kMaxStates before narrowing to nstates_.
  • Validating hdr.NumStates(), hdr.Start(), and hdr.NumArcs() as int64_t
    in ConstFstImpl::Read before assigning them to impl->nstates_,
    impl->start_, and impl->narcs_.
  • Expanding ConstLimitsTest in fst_limits_test.cc to test 64-bit values
    that truncate into 32-bit StateId against non-empty state streams.

@copybara-service
copybara-service Bot force-pushed the copybara/995324513 branch 2 times, most recently from 33cc626 to 84b45c4 Compare October 7, 2026 20:59
…s before narrowing.

In `ConstFstImpl<Arc, Unsigned>` (`lib/const-fst.h`):
1. `nstates_` and `start_` have type `StateId` (`int32_t` for `StdArc`, max
   `2^31 - 1`), whereas `kMaxStates` was `1ULL << 52`.
2. In `ConstFstImpl::Read`, the 64-bit `hdr.NumStates()`, `hdr.Start()`, and
   `hdr.NumArcs()` values were assigned to `impl->nstates_` (`StateId`),
   `impl->start_` (`StateId`), and `impl->narcs_` (`size_t`) before checking
   bounds. As a result, a 64-bit `hdr.NumStates()` or `hdr.Start()` such as
   `(1LL << 32) + 1` or `1LL << 32` silently truncated to `1` or `0` in 32-bit
   `StateId`, bypassing the range checks (`impl->nstates_ > kMaxStates` was
   tautologically false for 32-bit `StateId`), and `impl->narcs_ < 0` was an
   unsigned comparison.

Fix by:
- Making `kMaxStates` and `kMaxArcs` public on `ConstFstImpl` and capping
  `kMaxStates` at `std::min<uint64_t>(0x10000000000000ull, std::numeric_limits<StateId>::max())`.
- Counting states in `uint64_t` in `ConstFstImpl(const Fst<Arc>&)` and checking
  `nstates > kMaxStates` before narrowing to `nstates_`.
- Validating `hdr.NumStates()`, `hdr.Start()`, and `hdr.NumArcs()` as `int64_t`
  in `ConstFstImpl::Read` before assigning them to `impl->nstates_`,
  `impl->start_`, and `impl->narcs_`.
- Expanding `ConstLimitsTest` in `fst_limits_test.cc` to test 64-bit values
  that truncate into 32-bit `StateId` against non-empty state streams.

PiperOrigin-RevId: 995363968
@copybara-service
copybara-service Bot merged commit 3ac88d4 into main Oct 7, 2026
1 check passed
@copybara-service
copybara-service Bot deleted the copybara/995324513 branch October 7, 2026 21:40
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