Skip to content

Commit 807735c

Browse files
committed
fix(platform-config): read a file secret into one buffer at its bound
read_bounded grew a zeroizing Vec from empty while reading, and each growth copied the bytes read so far into a new buffer and freed the old one without erasing it, leaving prefixes of the secret in freed memory. The buffer is now allocated once at MAX_SECRET_BYTES + 1, which the bounded read never outgrows, so the value is held in one buffer that is erased on drop. Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
1 parent ca3afbf commit 807735c

1 file changed

Lines changed: 27 additions & 1 deletion

File tree

‎crates/registry-platform-config/src/secrets.rs‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,11 @@ fn read_secret_file(root: &Path, name: &str) -> Result<Zeroizing<Vec<u8>>, Secre
265265
read_bounded(file)
266266
}
267267

268+
/// Read at most one byte past [`MAX_SECRET_BYTES`] into one buffer allocated
269+
/// at that size, which the read never outgrows: a grown buffer would copy
270+
/// the value and free the old buffer without erasing it.
268271
fn read_bounded(file: File) -> Result<Zeroizing<Vec<u8>>, SecretError> {
269-
let mut bytes = Zeroizing::new(Vec::new());
272+
let mut bytes = Zeroizing::new(Vec::with_capacity(MAX_SECRET_BYTES + 1));
270273
file.take((MAX_SECRET_BYTES + 1) as u64)
271274
.read_to_end(&mut bytes)
272275
.map_err(|_| SecretError::Read)?;
@@ -455,6 +458,29 @@ mod tests {
455458
}
456459
}
457460

461+
/// A grown buffer copies the value and frees the old one unerased, so
462+
/// the read must fit one buffer allocated at the bound whatever the
463+
/// file holds.
464+
#[test]
465+
fn a_bounded_read_never_grows_its_buffer() {
466+
use std::io::{Seek as _, SeekFrom, Write as _};
467+
468+
for length in [
469+
1,
470+
4096,
471+
MAX_SECRET_BYTES,
472+
MAX_SECRET_BYTES + 1,
473+
2 * MAX_SECRET_BYTES,
474+
] {
475+
let mut file = tempfile::tempfile().expect("temporary file");
476+
file.write_all(&vec![b'x'; length]).expect("write");
477+
file.seek(SeekFrom::Start(0)).expect("rewind");
478+
let bytes = read_bounded(file).expect("read");
479+
assert_eq!(bytes.len(), length.min(MAX_SECRET_BYTES + 1));
480+
assert_eq!(bytes.capacity(), MAX_SECRET_BYTES + 1, "{length} bytes");
481+
}
482+
}
483+
458484
#[test]
459485
fn binary_values_keep_zero_bytes_within_the_same_bounds() {
460486
let secret = validate_binary_secret(Zeroizing::new(b"canary\0value".to_vec()))

0 commit comments

Comments
 (0)