Overflow when serializing weird values

[jean-airoldie]

While doing some fuzzing, I found this weird value that fails to serialize.

#[test]
fn it_can_serialize_weird_values() {
    let tests = [
        [1u8, 0, 30, 206, 97, 81, 216, 182, 20, 30, 165, 78, 18, 155, 169, 62],
    ];
    for &bytes in &tests {
        let dec = Decimal::deserialize(bytes);
        let bytes = bincode::serialize(&dec).unwrap();
        let dec2 = bincode::deserialize(&bytes).unwrap();
        assert_eq!(dec, dec2);
    }
}

Fails with:

---- it_can_serialize_weird_values stdout ----
thread 'it_can_serialize_weird_values' panicked at 'called `Result::unwrap()` on an `Err` value: CapacityError: insufficient capacity', /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/arrayvec-0.5.2/src/array_string.rs:165:26
stack backtrace:
   0: rust_begin_unwind
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/std/src/panicking.rs:515:5
   1: core::panicking::panic_fmt
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:92:14
   2: core::result::unwrap_failed
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:1599:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/result.rs:1281:23
   4: arrayvec::array_string::ArrayString<A>::push
             at /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/arrayvec-0.5.2/src/array_string.rs:165:9
   5: rust_decimal::str::to_str_internal
             at ./src/str.rs:60:13
   6: rust_decimal::serde::<impl serde::ser::Serialize for rust_decimal::decimal::Decimal>::serialize
             at ./src/serde.rs:177:34
   7: bincode::internal::serialized_size
             at /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/bincode-1.3.3/src/internal.rs:45:18
   8: bincode::internal::serialize
             at /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/bincode-1.3.3/src/internal.rs:31:27
   9: bincode::config::Options::serialize
             at /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/bincode-1.3.3/src/config/mod.rs:179:9
  10: bincode::serialize
             at /home/REDACTED/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/bincode-1.3.3/src/lib.rs:110:5
  11: decimal_tests::it_can_serialize_weird_values
             at ./tests/decimal_tests.rs:142:21
  12: decimal_tests::it_can_serialize_weird_values::{{closure}}
             at ./tests/decimal_tests.rs:136:1
  13: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5
  14: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[jean-airoldie]

I'm guessing that there are values that can be constructed via Decimal::deserialize that cannot be constructed via Decimal::from_str, which explains why the test suit didn't pick those up.

[jean-airoldie]

Seems like the internal display implementation can panic this way, which can cause some weird panicked while panicking issues.

rust-decimal/src/decimal.rs

Line 2056 in 805473b

let rep = crate::str::to_str_internal(self, false, f.precision());

[jean-airoldie]

This test makes the fmt::Display call panic as well:

#[test]
fn it_can_serialize_weird_values() {
    let tests = [
        [1u8, 0, 30, 206, 97, 81, 216, 182, 20, 30, 165, 78, 18, 155, 169, 62],
    ];
    for &bytes in &tests {
        let dec = Decimal::deserialize(bytes);
        let string = format!("{:.9999}", dec);
        let dec2 = Decimal::from_str(&string).unwrap();
        assert_eq!(dec, dec2);
    }
}

[paupino]

Great write up, thank you. While my immediate thought is to increase the string buffer size I've found that there is often another reason why the additional buffer is used. I'll need to some more investigation to see what is triggering this - hopefully later this week.

[koxu1996]

The following code fails as well:

let num = Decimal::from_str("1.2").unwrap();
println!("Decimal value:");
println!("{:.32}", num);

Decimal value:
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: CapacityError: insufficient capacity', /home/andrew/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/arrayvec-0.5.2/src/array_string.rs:165:26
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Process exited with code 101.

Not sure if this is expected.

[c410-f3r]

The following code fails as well:

let num = Decimal::from_str("1.2").unwrap();
println!("Decimal value:");
println!("{:.32}", num);

Decimal value:
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: CapacityError: insufficient capacity', /home/andrew/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/arrayvec-0.5.2/src/array_string.rs:165:26
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Process exited with code 101.

Not sure if this is expected.

Wow... Panicking inside the Display implementation was unexpected

[paupino]

This issue actually has two underlying problems present which should get fixed in #433 (once complete):

1. Display would panic when a precision greater than the string buffer was used (in this case, 30 for unsigned).
2. Deserialize supports scales greater than maximum precision. e.g. the scale in the buffer provided is 30 which is considered undefined behavior right now.