Skip to content

Commit 96a07e0

Browse files
committed
fix: Use Vec::resize() instead of set_len()
Otherwise it's possible for uninitialized memory to be used as if it was initialized, which can lead to strange behaviour. As the buffer is re-used, it's not actually zeroing that much memory either.
1 parent 46a4d4d commit 96a07e0

File tree

4 files changed

+9
-52
lines changed

4 files changed

+9
-52
lines changed

gix-filter/src/worktree/encode_to_git.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ pub(crate) mod function {
2626
use encoding_rs::DecoderResult;
2727

2828
use super::{Error, RoundTripCheck};
29-
use crate::clear_and_set_capacity;
3029

3130
/// Decode `src` according to `src_encoding` to `UTF-8` for storage in git and place it in `buf`.
3231
/// Note that the encoding is always applied, there is no conditional even if `src_encoding` already is `UTF-8`.
@@ -40,13 +39,8 @@ pub(crate) mod function {
4039
let buf_len = decoder
4140
.max_utf8_buffer_length_without_replacement(src.len())
4241
.ok_or(Error::Overflow { input_len: src.len() })?;
43-
clear_and_set_capacity(buf, buf_len);
44-
// SAFETY: `clear_and_set_capacity` assure that we have the given `buf_len` allocated, so setting its length is only making available
45-
// what is allocated. Later we will truncate to the amount of actually written bytes.
46-
#[allow(unsafe_code)]
47-
unsafe {
48-
buf.set_len(buf_len);
49-
}
42+
buf.clear();
43+
buf.resize(buf_len, 0);
5044
let (res, read, written) = decoder.decode_to_utf8_without_replacement(src, buf, true);
5145
match res {
5246
DecoderResult::InputEmpty => {
@@ -55,11 +49,7 @@ pub(crate) mod function {
5549
"encoding_rs estimates the maximum amount of bytes written correctly"
5650
);
5751
assert_eq!(read, src.len(), "input buffer should be fully consumed");
58-
// SAFETY: we trust that `encoding_rs` reports this number correctly, and truncate everything else.
59-
#[allow(unsafe_code)]
60-
unsafe {
61-
buf.set_len(written);
62-
}
52+
buf.truncate(written);
6353
}
6454
DecoderResult::OutputFull => {
6555
unreachable!("we assure that the output buffer is big enough as per the encoder's estimate")

gix-filter/src/worktree/encode_to_worktree.rs

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ pub(crate) mod function {
1717
use encoding_rs::EncoderResult;
1818

1919
use super::Error;
20-
use crate::clear_and_set_capacity;
2120

2221
/// Encode `src_utf8`, which is assumed to be UTF-8 encoded, according to `worktree_encoding` for placement in the working directory,
2322
/// and write it to `buf`, possibly resizing it.
@@ -33,13 +32,8 @@ pub(crate) mod function {
3332
.ok_or(Error::Overflow {
3433
input_len: src_utf8.len(),
3534
})?;
36-
clear_and_set_capacity(buf, buf_len);
37-
// SAFETY: `clear_and_set_capacity` assure that we have the given `buf_len` allocated, so setting its length is only making available
38-
// what is allocated. Later we will truncate to the amount of actually written bytes.
39-
#[allow(unsafe_code)]
40-
unsafe {
41-
buf.set_len(buf_len);
42-
}
35+
buf.clear();
36+
buf.resize(buf_len, 0);
4337
let src = std::str::from_utf8(src_utf8)?;
4438
let (res, read, written) = encoder.encode_from_utf8_without_replacement(src, buf, true);
4539
match res {
@@ -49,11 +43,7 @@ pub(crate) mod function {
4943
"encoding_rs estimates the maximum amount of bytes written correctly"
5044
);
5145
assert_eq!(read, src_utf8.len(), "input buffer should be fully consumed");
52-
// SAFETY: we trust that `encoding_rs` reports this number correctly, and truncate everything else.
53-
#[allow(unsafe_code)]
54-
unsafe {
55-
buf.set_len(written);
56-
}
46+
buf.truncate(written);
5747
}
5848
EncoderResult::OutputFull => {
5949
unreachable!("we assure that the output buffer is big enough as per the encoder's estimate")

gix-pack/src/cache/delta/traverse/resolve.rs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ where
122122
let (result_size, consumed) = data::delta::decode_header_size(&delta_bytes[consumed..]);
123123
header_ofs += consumed;
124124

125-
set_len(fully_resolved_delta_bytes, result_size as usize);
125+
fully_resolved_delta_bytes.resize(result_size as usize, 0);
126126
data::delta::apply(&base_bytes, fully_resolved_delta_bytes, &delta_bytes[header_ofs..]);
127127

128128
// FIXME: this actually invalidates the "pack_offset()" computation, which is not obvious to consumers
@@ -394,28 +394,13 @@ where
394394
})
395395
}
396396

397-
fn set_len(v: &mut Vec<u8>, new_len: usize) {
398-
if new_len > v.len() {
399-
v.reserve_exact(new_len.saturating_sub(v.capacity()) + (v.capacity() - v.len()));
400-
// SAFETY:
401-
// 1. we have reserved enough capacity to fit `new_len`
402-
// 2. the caller is trusted to write into `v` to completely fill `new_len`.
403-
#[allow(unsafe_code, clippy::uninit_vec)]
404-
unsafe {
405-
v.set_len(new_len);
406-
}
407-
} else {
408-
v.truncate(new_len)
409-
}
410-
}
411-
412397
fn decompress_all_at_once_with(
413398
inflate: &mut zlib::Inflate,
414399
b: &[u8],
415400
decompressed_len: usize,
416401
out: &mut Vec<u8>,
417402
) -> Result<(), Error> {
418-
set_len(out, decompressed_len);
403+
out.resize(decompressed_len, 0);
419404
inflate.reset();
420405
inflate.once(b, out).map_err(|err| Error::ZlibInflate {
421406
source: err,

gix-worktree-stream/src/protocol.rs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,5 @@ fn mode_to_byte(m: gix_object::tree::EntryMode) -> u8 {
126126

127127
fn clear_and_set_len(buf: &mut Vec<u8>, len: usize) {
128128
buf.clear();
129-
if buf.capacity() < len {
130-
buf.reserve(len);
131-
assert!(buf.capacity() >= len, "{} >= {}", buf.capacity(), len);
132-
}
133-
// SAFETY: we just assured that `buf` has the right capacity to hold `cap`
134-
#[allow(unsafe_code)]
135-
unsafe {
136-
buf.set_len(len);
137-
}
129+
buf.resize(len, 0);
138130
}

0 commit comments

Comments
 (0)