Skip to content

Commit 0dc2a0c

Browse files
committed
fix: prevent newline amplification
A fuzzer found input sized 780kb, but the output becomes 180MB in size, ultimately taking 48s to parse on M1 Pro. Note that the parsing itself is fast, but we seem to modify the input in such a way that parsing takes way longer than it should. That might be another issue which it will probably find next, let's wait for it.
1 parent 6c0364a commit 0dc2a0c

File tree

4 files changed

+17
-8
lines changed

4 files changed

+17
-8
lines changed

gix-config/src/file/write.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,19 @@ pub(crate) fn ends_with_newline(e: &[crate::parse::Event<'_>], nl: impl AsRef<[u
8383
}
8484

8585
pub(crate) fn extract_newline<'a>(e: &'a Event<'_>) -> Option<&'a BStr> {
86-
match e {
87-
Event::Newline(b) => b.as_ref().into(),
88-
_ => None,
89-
}
86+
Some(match e {
87+
Event::Newline(b) => {
88+
let nl = b.as_ref();
89+
90+
// Newlines are parsed consecutively, be sure we only take the smallest possible variant
91+
if nl.contains(&b'\r') {
92+
"\r\n".into()
93+
} else {
94+
"\n".into()
95+
}
96+
}
97+
_ => return None,
98+
})
9099
}
91100

92101
pub(crate) fn platform_newline() -> &'static BStr {

gix-config/src/parse/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ pub enum Event<'a> {
5959
Value(Cow<'a, BStr>),
6060
/// Represents any token used to signify a newline character. On Unix
6161
/// platforms, this is typically just `\n`, but can be any valid newline
62-
/// sequence. Multiple newlines (such as `\n\n`) will be merged as a single
62+
/// *sequence*. Multiple newlines (such as `\n\n`) will be merged as a single
6363
/// newline event containing a string of multiple newline characters.
6464
Newline(Cow<'a, BStr>),
6565
/// Any value that isn't completed. This occurs when the value is continued

gix-config/tests/file/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ fn fuzzed_long_runtime() -> crate::Result {
4949
let config = std::fs::read(fixture_path_standalone("fuzzed/long-parsetime.config"))?;
5050
let file = File::from_bytes_no_includes(&config, gix_config::file::Metadata::default(), Default::default())?;
5151
assert_eq!(file.sections().count(), 52);
52-
assert_eq!(file.to_bstring().len(), 180351853);
52+
assert!(file.to_bstring().len() < 1200000);
5353
File::from_bytes_no_includes(
5454
&file.to_bstring(),
5555
gix_config::file::Metadata::default(),
@@ -60,7 +60,7 @@ fn fuzzed_long_runtime() -> crate::Result {
6060
mutated_file.append(file);
6161
assert_eq!(mutated_file.sections().count(), 52 * 2);
6262
let serialized = mutated_file.to_bstring();
63-
assert_eq!(serialized.len(), 360703706);
63+
assert!(serialized.len() < 2400000);
6464
File::from_bytes_no_includes(&serialized, gix_config::file::Metadata::default(), Default::default())?;
6565
Ok(())
6666
}

gix-config/tests/mem.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ fn usage() {
2727
peak = ByteSize(ALLOCATOR.max_allocated() as u64),
2828
);
2929
assert!(
30-
used < 60 * 1024 * 1024,
30+
used < 80 * 1024 * 1024,
3131
"we should now start using more memory than anticipated, to keep mem-amplification low"
3232
);
3333
}

0 commit comments

Comments
 (0)