Skip to content

Fix high-severity code scanning issues in geo_filters, hash-sorted-map, and bpe - #175

Merged
aneubeck merged 12 commits into
mainfrom
aneubeck-triage-rust-gems-security-alerts
Nov 4, 2026
Merged

aneubeck merged 12 commits into
mainfrom
aneubeck-triage-rust-gems-security-alerts

Conversation

@aneubeck

@aneubeck aneubeck commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Addresses all 6 high-severity ai-scan/* code-scanning alerts as well as medium-severity alert code-scanning/14 in BitVec::from_bytes:

  • geo_filters:
    • code-scanning/11 (ai-scan/cwe-704, crates/geo_filters/src/diff_count.rs:380) & code-scanning/13 (ai-scan/cwe-704, crates/geo_filters/src/diff_count/bitvec.rs:198): GeoDiffCount::from_bytes_with_config and BitVec::from_bytes previously transmuted &[u8] to &[C::BucketType] and &[u64] without verifying pointer alignment (and after advancing buf by 1 header byte in BitVec::from_bytes). Introduce a 1-byte-aligned #[repr(C, packed)] Unaligned<T> wrapper in crates/geo_filters/src/config/unaligned.rs so GeoDiffCount::msb and BitVec::blocks can borrow arbitrary byte slices zero-copy (Cow::Borrowed) without alignment UB.
    • code-scanning/14 (ai-scan/cwe-191, crates/geo_filters/src/diff_count/bitvec.rs:190): Check that BitVec::from_bytes has a non-empty multiple of 8 bytes after the header byte before computing num_bits, preventing integer underflow on 1-byte inputs.
    • Tie the lifetime of BitVec::from_bytes to its input slice (buf: &'a [u8]).
  • hash-sorted-map:
    • code-scanning/9 (ai-scan/cwe-415, crates/hash-sorted-map/src/hash_sorted_map.rs:220): HashSortedMap::sort_by_hash previously moved cur_key and cur_val out of MaybeUninit via assume_init_read before the Ord::cmp insertion-sort loop while leaving ctrl marked occupied, risking a double-drop if Ord::cmp panicked. Borrow cur_key in place during comparisons and only move cur_key/cur_val (along with their ctrl tags) inside if j < i after all comparisons for the element succeed.
    • code-scanning/12 (ai-scan/cwe-119, crates/hash-sorted-map/src/group_ops.rs:116): Use from_le_bytes in group_ops (match_tag and count_occupied) and gate aarch64 NEON bitmask extraction on target_endian = "little" so slot index bit positions match trailing_zeros/leading_zeros on big-endian targets.
  • bpe:
    • code-scanning/8 (ai-scan/cwe-502, crates/bpe/src/byte_pair_encoding.rs:75): Validate serialized DoubleArrayAhoCorasick<u32> buffers in DaacVisitor::visit_bytes (states_len multiple of 256, byte lengths, match_kind, num_states, state base/fail/output_pos bounds, and output length/parent bounds) before calling DoubleArrayAhoCorasick::deserialize_unchecked.
    • code-scanning/15 (ai-scan/cwe-125, crates/bpe/src/byte_pair_encoding.rs:140): Replace unchecked indexing (get_unchecked) in token_range, is_valid_token_pair, encode_all_prefixes, AppendableEncoder::push, and PrependableEncoder::push with safe bounds-checked indexing.

Relates to github/blackbird#17346

aneubeck and others added 3 commits November 4, 2026 09:58
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aneubeck
aneubeck requested a review from a team as a code owner November 4, 2026 08:59
Copilot AI auto review requested due to automatic review settings November 4, 2026 08:59
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Malformed dictionaries can hang encoding through cyclic failure links or split tables, new unwraps violate CI lint rules, and comparison panics can leave map entries inaccessible.

4 open issues
What changed in this PR

Hardens geo_filters, hash-sorted-map, and bpe against code-scanning issues involving binary decoding, unsafe indexing, and panic-related ownership errors.

Changes:

  • Add alignment-aware filter decoding and validate BitVec payload lengths.
  • Delay map entry moves until comparisons succeed and correct endian-dependent control-byte handling.
  • Add serialized BPE automaton validation, replace unchecked indexing, and introduce regression tests.
File Description
crates/​hash-sorted-map/​src/​hash_sorted_map.rs Delay ownership moves during sorting and test drop behavior after a comparison panic.
crates/​hash-sorted-map/​src/​group_ops.rs Make control-byte decoding portable across byte orders.
crates/​geo_filters/​src/​diff_count/​bitvec.rs Correct input lifetimes, handle unaligned data, and reject invalid payload lengths.
crates/​geo_filters/​src/​diff_count.rs Borrow aligned bucket data or copy unaligned data into owned storage.
crates/​bpe/​src/​prependable_encoder.rs Replace unchecked state access with bounds-checked indexing.
crates/​bpe/​src/​byte_pair_encoding.rs Validate automaton bytes, bounds-check token access, and add regression tests.
crates/​bpe/​src/​appendable_encoder.rs Replace unchecked state access with bounds-checked indexing.

Review effort: Max (auto)

Comment thread crates/bpe/src/byte_pair_encoding.rs Resolved
Comment thread crates/bpe/src/byte_pair_encoding.rs Resolved
Comment thread crates/bpe/src/byte_pair_encoding.rs Resolved
aneubeck and others added 7 commits November 4, 2026 10:36
Store BitVec blocks as Cow<'a, [[u8; BYTES_PER_BLOCK]]> and read/write u64
values via u64::from_le_bytes/to_le_bytes so BitVec::from_bytes always borrows
directly from the input slice without copying or unsafe code.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Store GeoDiffCount::msb as Cow<[<C::BucketType as IsBucketType>::Bytes]>
and decode/encode bucket values via IsBucketType::from_le_bytes and
to_le_bytes so GeoDiffCount::from_bytes_with_config and write are
zero-copy without requiring BucketType alignment or unsafe code.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Convert bytes[offset..offset + 4] into [u8; 4] via try_into().expect(...)
instead of indexing 4 elements individually.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Move msb and lsb comparisons into the Ordering::Equal branches of the
preceding match expressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aneubeck
aneubeck merged commit 0299c00 into main Nov 4, 2026
9 checks passed
@aneubeck
aneubeck deleted the aneubeck-triage-rust-gems-security-alerts branch November 4, 2026 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants