crash due to absurd memory allocation

[alexanderkjall]

Some config file generate an extreme memory allocation when read, that leads to a crash.

Example program that generates the error:

use config::{Config, File, FileFormat};

fn main() {
    let s = vec![0x70, 0x4d, 0x5b, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32,
                 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x5d, 0x3a, 0x00, 0x00,
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x33, 0x03, 0x00,
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00];

    let _ = Config::new().merge(File::from_str(std::str::from_utf8(&s).unwrap(), FileFormat::Ini));
}

Output:

$ RUST_BACKTRACE=1 ./target/debug/config-reproduce 
memory allocation of 1777777777777777840 bytes failed
Aborted (core dumped)

Stacktrace:

(gdb) bt
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:49
#1  0x00007ffff7dc8864 in __GI_abort () at abort.c:79
#2  0x0000555555747e67 in std::sys::unix::abort_internal () at library/std/src/sys/unix/mod.rs:255
#3  0x0000555555581476 in std::process::abort () at library/std/src/process.rs:1957
#4  0x000055555574440e in std::alloc::rust_oom () at library/std/src/alloc.rs:330
#5  0x000055555575bb67 in alloc::alloc::__alloc_error_handler::__rg_oom () at library/alloc/src/alloc.rs:398
#6  0x0000555555582237 in alloc::alloc::handle_alloc_error () at library/alloc/src/alloc.rs:367
#7  0x00005555555fafdc in alloc::raw_vec::handle_reserve (result=...) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/raw_vec.rs:531
#8  0x00005555555fbb11 in alloc::raw_vec::RawVec<T,A>::reserve::do_reserve_and_handle (slf=0x5555557f2bb0, len=0, additional=22222222222222223) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/raw_vec.rs:333
#9  0x00005555555fbacb in alloc::raw_vec::RawVec<T,A>::reserve (self=0x5555557f2bb0, len=0, additional=22222222222222223) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/raw_vec.rs:337
#10 0x0000555555602cd1 in alloc::vec::Vec<T,A>::reserve (self=0x5555557f2bb0, additional=22222222222222223) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/vec/mod.rs:803
#11 0x0000555555602492 in alloc::vec::Vec<T,A>::extend_with (self=0x5555557f2bb0, n=22222222222222223, value=...) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/vec/mod.rs:2226
#12 0x0000555555602c66 in alloc::vec::Vec<T,A>::resize (self=0x5555557f2bb0, new_len=22222222222222223, value=...) at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/alloc/src/vec/mod.rs:2122
#13 0x00005555555ee99d in config::path::Expression::set (self=0x7fffffffca10, root=0x7fffffffcbb8, value=...) at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/path/mod.rs:245
#14 0x00005555555848f6 in config::source::Source::collect_to (self=0x5555557f1e10, cache=0x7fffffffcbb8) at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/source.rs:27
#15 0x0000555555603af0 in <alloc::vec::Vec<alloc::boxed::Box<dyn config::source::Source+core::marker::Send+core::marker::Sync>> as config::source::Source>::collect (self=0x7fffffffdae0)
    at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/source.rs:53
#16 0x000055555560357c in config::source::Source::collect_to (self=0x7fffffffdae0, cache=0x7fffffffd380) at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/source.rs:17
#17 0x00005555555f9e20 in config::config::Config::refresh (self=0x7fffffffda80) at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/config.rs:124
#18 0x0000555555583f16 in config::config::Config::merge (self=0x7fffffffda80, source=...) at /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/config-0.11.0/src/config.rs:78
#19 0x0000555555583a53 in config_reproduce::main () at src/main.rs:12

[matthiasbeyer]

So first of all I'd like to understand why you think that this sequence of bytes represents a config file? Did this come out of some fuzzying tool?

[alexanderkjall]

Yes, I'm evaluating if it would be reasonable to send users config files from other users in my program.

The program is https:/cortex/ripasso/ and it's a password manager, that can be used in a group setting, and having a common config file that is shared between the group would be useful.

[matthiasbeyer]

While I understand your usecase... wouldn't it be better if some sync mechanism would do this (think git or syncthing) instead of reinventing the wheel in your program?

[alexanderkjall]

Yes, the transportation layer for the config file would be git in my case.

The (highly theoretical) scenario would be if one of the users checked in a config file that would case some sort of problem for the other users, the first users computer could get hacked for example, and I wouldn't want an attacker to be able to move laterally to the other users machines by committing a payload to the config file.

I fully realize that this is an extremely hypothetical scenario, but the reason why I use rust is that it's a more language than c/c++.

And that's why I have been going over all my dependencies with a fuzzer and reporting the bugs that I have found.

A crash due to a large memory allocation isn't really a security problem, at most it would be a DoS, but it's kind of annoying if it happens and would feel like a bug, so I thought that it warranted a bug report at least :)

[conradludgate]

It's not a real solution, but if random non-text characters are causing issues, you could try a simple heuristic like https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=d5ca35753f54ac7902c40a571acfa1a9

Is this a problem in serde ini or in config?

[matthiasbeyer]

Hi. Does this issue still exist in later versions of this crate?

[alexanderkjall]

Hi, it seems like that, had to modify the above program slightly to reproduce the issue:

use config::{Config, File, FileFormat};

fn main() {
    let s = vec![0x70, 0x4d, 0x5b, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32,
                 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x32, 0x5d, 0x3a, 0x00, 0x00,
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x33, 0x03, 0x00,
                 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00];

 	let	_ = Config::builder().build().unwrap().merge(File::from_str(std::str::from_utf8(&\
s).unwrap(), FileFormat::Ini));
}

produced this output:

[capitol@batia config-crash]$ cargo run
   Compiling config-crash v0.1.0 (/tmp/config-crash)
warning: use of deprecated associated function `config::Config::merge`: please use 'ConfigBuilder' instead
 --> src/main.rs:9:48
  |
9 |     let _ = Config::builder().build().unwrap().merge(File::from_str(std::str::from_utf8(&s).unwrap(), FileFormat::Ini));
  |                                                ^^^^^
  |
  = note: `#[warn(deprecated)]` on by default

warning: `config-crash` (bin "config-crash") generated 1 warning
    Finished dev [unoptimized + debuginfo] target(s) in 0.45s
     Running `target/debug/config-crash`
memory allocation of 1777777777777777840 bytes failed
Avbrutt (SIGABRT) (kjerne lagret i fil)

[matthiasbeyer]

So the byte array above is parsed to this expression:

pM[22222222222222222]3

Which is then parsed as a expression indexing pM at 22222222222222222 with config::path::Expression::Subscript(/* some hex */, 22222222222222222).

That index is then used later to grow the cache for indexing the loaded documents. That is of course an absurd size...

I am not sure yet where to fix this (or whether to fix this at all because this is really obscure). 🤔 Still, non-segfaulting software is always better than segfaulting one, of course, so I think we should fix this even for the sake of it.
But as said, I am not even sure how and where...

[alexanderkjall]

One solution for the future could maybe be to use the try_reserve functions from rust-lang/rust#48043

Until that is merged I'm not sure that there is a good way to handle this class of problem, since it's hard to know compile-time how memory constrained the running environment will be.

Feel free to close this if you want :)