Skip to content

fix(rust): harden network block framing#2294

Open
GautamSharma99 wants to merge 1 commit into
CCExtractor:masterfrom
GautamSharma99:fix/rust-network-framing
Open

fix(rust): harden network block framing#2294
GautamSharma99 wants to merge 1 commit into
CCExtractor:masterfrom
GautamSharma99:fix/rust-network-framing

Conversation

@GautamSharma99

Copy link
Copy Markdown

Summary

This PR hardens the Rust networking implementation against fragmented TCP I/O and oversized protocol blocks.

The existing implementation assumed that a single TcpStream::read() or TcpStream::write() transferred the entire requested
buffer. TCP does not guarantee this: successful operations may process only part of the supplied buffer.

A partial operation could leave the custom network protocol out of sync because the next call would interpret the remainder of the
previous block as a new command.

This PR also prevents peer-provided block lengths from causing an unbounded allocation or an out-of-bounds copy into the buffer
supplied through the C/Rust FFI boundary.

Problem

CCExtractor's network protocol encodes blocks as:

command | length | data | \r\n
1 byte  | 10 B   | N B  | 2 B

The Rust implementation previously performed one socket operation for each section:

self.send(data)?;
self.recv(&mut data)?;

This was unsafe for TCP framing because:

- read() can return fewer bytes than requested.
- write() can write fewer bytes than requested.
- interrupted operations were not retried.
- a short operation could consume part of a block before returning, leaving the stream misaligned.
- the received length was used directly in vec![0; length].
- received data was copied into the FFI caller's buffer without verifying that it fit.

The legacy C implementation already handles complete transfers through readn() and writen() and limits received data to the caller-
provided buffer capacity.

## Changes

### Complete framed transfers

Added internal send_all() and recv_exact() helpers to BlockStream.

These helpers:

- continue after partial reads and writes;
- retry operations interrupted with ErrorKind::Interrupted;
- distinguish EOF or zero progress from a completed transfer;
- return existing NetError values for socket errors.

send_block() and recv_block() now use these helpers for every protocol section.

### Bounded block reception

recv_block() now accepts the maximum permitted data length.

When a peer announces a block larger than that limit:

1. the payload is consumed in fixed-size chunks without allocating the announced size;
2. the end marker is validated;
3. an error is returned;
4. the stream remains aligned so a subsequent valid block can still be read.

The destination buffer length from net_tcp_read() is passed down as this limit.

### Checked FFI copying

The final copy into the caller-provided buffer now uses a checked slice lookup.

Although the block decoder already enforces the destination capacity, this adds a final guard at the FFI-facing layer and prevents a
Rust slice panic if those assumptions change later.

### Reliable writes with nonblocking ping polling

SendTarget previously made the entire TCP stream nonblocking immediately after connecting. This also made password negotiation,
description transmission, and caption block writes susceptible to partial writes or WouldBlock.

Normal protocol writes now use the blocking stream.

Connection health checks temporarily enable nonblocking mode only while draining pending one-byte Ping commands, then restore
blocking mode. This matches the behavior of the legacy C implementation, where ping polling is nonblocking but framed writes are
completed synchronously.

## Tests

Added regression coverage for:

- partial writes;
- partial reads;
- interrupted reads and writes;
- EOF in the middle of a frame;
- invalid length fields;
- oversized frame rejection;
- draining an oversized frame without desynchronizing the next frame;
- copying data that fits the FFI destination;
- rejecting data larger than the FFI destination.

## Validation

### Rust library tests

cargo test
56 passed; 0 failed

### Rust library documentation tests

45 passed; 0 failed

### Main Rust crate tests

cargo test
400 passed; 0 failed

### Formatting

cargo fmt --all -- --check

Passed for both src/rust and src/rust/lib_ccxr.

### Clippy

cargo clippy -- -D warnings

Passed for both Rust crates.

### Production build

A complete CMake build, including the C code and release-mode Rust static library, completed successfully:

[100%] Built target ccextractor

The resulting binary also passed a --version smoke test.

## Legacy C test-suite note

The legacy tests/Makefile target was attempted, but its existing test source no longer compiles against the current codebase. It
references:

- an undeclared sbs_init_context() function;
- the removed cc_bitmap.data member.

These failures occur in unchanged split-by-sentence test code before this networking change is linked. No legacy C test files were
modified as part of this PR.

## Scope

This PR only changes the Rust networking module:

- src/rust/lib_ccxr/src/net/block.rs
- src/rust/lib_ccxr/src/net/c_functions.rs
- src/rust/lib_ccxr/src/net/source.rs
- src/rust/lib_ccxr/src/net/target.rs

No protocol format changes were introduced, so the Rust and legacy C implementations remain wire-compatible.

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.

1 participant