Russh: Unchecked CryptoVec allocation and growth handling is reachable
Title
UncheckedCryptoVec allocation and growth handling was reachable from local agent inputs in current russh releases and from remote SSH traffic in historical pre-0.58.0 releasesSummary
CryptoVec used unchecked capacity growth, unchecked length arithmetic, and unsafe allocation/locking paths. In current russh releases, local SSH agent peers could still feed attacker-controlled frame lengths into buffer growth before validation. In older russh releases before 0.58.0, remote SSH traffic also reached CryptoVec through transport and compression buffers.Details
The underlying unsafe paths were inCryptoVec:
cryptovec/src/cryptovec.rs- unchecked capacity growth
- unchecked length arithmetic in growth callers
- raw allocation and reallocation paths coupled to those sizes
cryptovec/src/platform/unix.rsmlock/munlockpreviously accepted zero-length calls and performed null-pointer validation inside theunsafeOS-call path
There are two relevant reachability stories:
- current local reachability in
russh russh/src/keys/agent/client.rsAgentClient::read_response()read a peer-suppliedu32length and then resizedself.bufto that value before reading the payloadrussh/src/keys/agent/server.rsConnection::run()read a peer-suppliedu32length and then resizedself.bufto that value before reading the payload
This is the path that still existed in current 0.60.x releases before the fix, although by then those buffers were no longer CryptoVec.
- historical remote reachability in older
russh - before commit
712e32b(first released inv0.58.0), non-secret transport and compression buffers inrusshstill usedCryptoVec - I verified this in a detached pre-
712e32bworktree by adding and running: cipher::tests::remote_packet_length_grows_transport_cryptovec_buffercompression::tests::remote_compressed_payload_expands_cryptovec_output- those tests show that remote SSH traffic could grow
CryptoVecthrough: - transport packet reads
- zlib decompression output
Also added a constrained-memory reproduction in that historical worktree:
compression::tests::remote_compressed_payload_can_crash_under_memory_limit
That test re-execs the test binary under prlimit --as=134217728, decompresses a highly compressible payload that expands to 96 MiB, and reliably aborts in the old Unix CryptoVec path when NonNull::new_unchecked() receives a null pointer after allocation failure.
The prepared patch does two things:
- hardens
CryptoVecitself - checked capacity growth
- checked length arithmetic
- immediate allocation-failure handling
- zero-length
mlock/munlockno-ops - explicit null-pointer validation before entering the Unix
unsafelocking calls - hardens the real untrusted-input path
- caps agent frame lengths at
256 * 1024on both client and server before resizing buffers
This cap matches OpenSSH’s agent framing guardrail.
PoC
The following end-to-end tests demonstrate the real untrusted-input path by feeding oversized peer-controlled agent frame lengths into the public client and server flows and asserting that they are rejected before buffer growth.Client-side agent reply path:
#[test]
fn oversized_agent_response_is_rejected_before_allocation() -> std::io::Result<()> {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()?; runtime.block_on(async {
let (mut writer, reader) = tokio::io::duplex(64);
let server = tokio::spawn(async move {
let mut frame = [0u8; 4];
writer.read_exact(&mut frame).await?;
let len = BigEndian::read_u32(&frame) as usize;
let mut body = vec![0; len];
writer.read_exact(&mut body).await?;
BigEndian::write_u32(&mut frame, (MAX_AGENT_FRAME_LEN + 1) as u32);
writer.write_all(&frame).await?;
Ok::<(), std::io::Error>(())
});
let mut client = AgentClient::connect(reader);
let err = client.request_identities().await.unwrap_err();
assert!(matches!(err, Error::AgentProtocolError));
server.await.expect("server task")?;
Ok::<(), std::io::Error>(())
})?;
Ok(())
}
Server-side agent request path:
#[test]
fn oversized_agent_request_is_rejected_before_allocation() -> std::io::Result<()> {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()?; runtime.block_on(async {
let (server, mut client) = tokio::io::duplex(64);
let connection = Connection {
lock: Lock(std::sync::Arc::new(std::sync::RwLock::new(crate::CryptoVec::new()))),
keys: KeyStore(std::sync::Arc::new(std::sync::RwLock::new(
std::collections::HashMap::new(),
))),
agent: Some(()),
s: server,
buf: Vec::new(),
};
let server = tokio::spawn(async move { connection.run().await });
let mut frame = [0u8; 4];
BigEndian::write_u32(&mut frame, (MAX_AGENT_FRAME_LEN + 1) as u32);
client.write_all(&frame).await?;
drop(client);
let err = server.await.expect("server task").unwrap_err();
assert!(matches!(err, Error::AgentProtocolError));
Ok::<(), std::io::Error>(())
})?;
Ok(())
}
These tests pass on the fixed branch and fail on unfixed v0.60.2, where oversized agent frame lengths are not rejected at the framing boundary.
For historical russh < 0.58.0, I also verified remote reachability into CryptoVec in a detached pre-712e32b worktree (91d431d, package version 0.57.1).
Transport packet read path:
#[test]
fn remote_packet_length_grows_transport_cryptovec_buffer() -> std::io::Result<()> {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()?; runtime.block_on(async {
let packet_len = MAXIMUM_PACKET_LEN;
let (mut writer, mut reader) = tokio::io::duplex(packet_len + 4);
let writer_task = tokio::spawn(async move {
let mut packet = vec![0u8; packet_len + 4];
packet[..4].copy_from_slice(&(packet_len as u32).to_be_bytes());
writer.write_all(&packet).await?;
Ok::<(), std::io::Error>(())
});
let mut buffer = SSHBuffer::new();
let mut cipher = clear::Key;
let n = read(&mut reader, &mut buffer, &mut cipher).await.unwrap();
assert_eq!(n, packet_len + 4);
assert_eq!(buffer.buffer.len(), packet_len + 4);
assert_eq!(&buffer.buffer[..4], &(packet_len as u32).to_be_bytes());
writer_task.await.expect("writer task")?;
Ok::<(), std::io::Error>(())
})?;
Ok(())
}
Compression growth path:
#[test]
fn remote_compressed_payload_expands_cryptovec_output() {
let payload = vec![b'A'; 64 * 1024]; let compression = Compression::new(&ZLIB);
let mut compressor = Compress::None;
let mut decompressor = Decompress::None;
compression.init_compress(&mut compressor);
compression.init_decompress(&mut decompressor);
let mut compressed = CryptoVec::new();
let encoded = compressor
.compress(&payload, &mut compressed)
.expect("compress")
.to_vec();
let mut output = CryptoVec::new();
let decoded = decompressor
.decompress(&encoded, &mut output)
.expect("decompress");
assert_eq!(decoded.len(), payload.len());
assert_eq!(decoded, payload.as_slice());
assert!(encoded.len() < output.len());
}
Constrained-memory crash reproduction for the historical remote compression path:
```rust #[test] fn remote_compressed_payload_can_crash_under_memory_limit() { const CHILD_ENV: &str = "RUSSH_REMOTE