[BUG]

[Geal]

Description

Parsing long numbers can result in a panic. This was reported in nom: rust-bakery/nom#1421
Parsing a text like "0.00000000000000000087" results in this stacktrace:

---- number::complete::tests::float_panic stdout ----
thread 'number::complete::tests::float_panic' panicked at 'attempt to shift left with overflow', /home/geal/.cargo/reg
istry/src/github.com-1ecc6299db9ec823/minimal-lexical-0.2.1/src/lemire.rs:155:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/5ecc8ad8462574354a55162a0c16b10eb95e3e70/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/5ecc8ad8462574354a55162a0c16b10eb95e3e70/library/core/src/panicking.rs:103:14
   2: core::panicking::panic
             at /rustc/5ecc8ad8462574354a55162a0c16b10eb95e3e70/library/core/src/panicking.rs:50:5
   3: minimal_lexical::lemire::compute_error
             at /home/geal/.cargo/registry/src/github.com-1ecc6299db9ec823/minimal-lexical-0.2.1/src/lemire.rs:155:5
   4: minimal_lexical::lemire::lemire
             at /home/geal/.cargo/registry/src/github.com-1ecc6299db9ec823/minimal-lexical-0.2.1/src/lemire.rs:25:14
   5: minimal_lexical::parse::moderate_path
             at /home/geal/.cargo/registry/src/github.com-1ecc6299db9ec823/minimal-lexical-0.2.1/src/parse.rs:178:12
   6: minimal_lexical::parse::parse_float
             at /home/geal/.cargo/registry/src/github.com-1ecc6299db9ec823/minimal-lexical-0.2.1/src/parse.rs:160:18
   7: nom::number::complete::float
             at ./src/number/complete.rs:1551:24

I'm wondering if this should be solved in nom or minimal-lexical? I'm not sure nom has enough info to evaluate if the float will be too large

[Alexhuszagh]

Let me quickly check: this is specifically an issue with a number with a large number of zeros and not a large number of digits in general. Can you tell me how this is passed to minimal-lexical?

Is it: parse_float(b"0".iter(), b"00000000000000000087".iter(), 0), or parse_float(b"".iter(), b"00000000000000000087".iter(), 0). It seems like an edge-case in minimal-lexical, however, it may be an issue elsewhere as well. There's 19 leading zeros with the integral zero as well, which is the maximum number of digits you can always parse in a double-precision float.

Specifically, the issue is here:

minimal-lexical/src/lemire.rs

Lines 153 to 158 in e997c46

	pub fn compute_error<F: Float>(q: i32, mut w: u64) -> ExtendedFloat {
	let lz = w.leading_zeros() as i32;
	w <<= lz;
	let hi = compute_product_approx(q, w, F::MANTISSA_SIZE as usize + 3).1;
	compute_error_scaled::<F>(q, hi, lz)
	}

Which can only fail if w = 0, where w is the significant digits.

[Alexhuszagh]

This is likely a bug in minimal-lexical, since our documentation clearly states we should not have leading zeros only in the integer component.

[nbigaouette]

I got the same panic when using nom 7.0.0 and this input: b"0.00000000000000000018".

[nbigaouette]

This bug unfortunately prevents me from fuzzing my code (libfuzzer always converges to it, damn it's good! 😂 )

I can add a test to the crate's test suite to reproduce (over e997c46):

diff --git a/tests/parse_tests.rs b/tests/parse_tests.rs
index 48856fd..91d24ab 100644
--- a/tests/parse_tests.rs
+++ b/tests/parse_tests.rs
@@ -51,6 +51,8 @@ fn parse_f32_test() {
     check_parse_float("1", "00000017881393432617187501", 0, 1.0000002_f32);

     check_parse_float("", "000000000000000000000000000000000000011754943508222875079687365372222456778186655567720875215087517062784172594547271728515625", 0, 1.1754943508222875e-38f32);
+
+    check_parse_float("0", "00000000000000000018", 0, 0.00000000000000000018_f32);
 }

 #[test]

Running the tests in debug mode I get the panic:

❯ cargo test parse_f32_test
[...]
running 1 test
test parse_f32_test ... FAILED

failures:

---- parse_f32_test stdout ----
thread 'parse_f32_test' panicked at 'attempt to shift left with overflow', ~/minimal-lexical.git/src/lemire.rs:155:5
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::panicking::panic
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:50:5
   3: minimal_lexical::lemire::compute_error
             at ./src/lemire.rs:155:5
   4: minimal_lexical::lemire::lemire
             at ./src/lemire.rs:25:14
   5: minimal_lexical::parse::moderate_path
             at ./src/parse.rs:178:12
   6: minimal_lexical::parse::parse_float
             at ./src/parse.rs:160:18
   7: parse_tests::check_parse_float
             at ./tests/parse_tests.rs:7:25
   8: parse_tests::parse_f32_test
             at ./tests/parse_tests.rs:55:5
   9: parse_tests::parse_f32_test::{{closure}}
             at ./tests/parse_tests.rs:11:1
  10: core::ops::function::FnOnce::call_once
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/ops/function.rs:227:5
  11: 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.

In release mode though the parsing succeeds, but the float comparison fails:

❯ cargo test parse_f32_test --release
[...]
test parse_f32_test ... FAILED

failures:

---- parse_f32_test stdout ----
thread 'parse_f32_test' panicked at 'assertion failed: expected == parse::parse_float::<F, _, _>(integer, fraction, exponent)', tests/parse_tests.rs:7:5
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::panicking::panic
             at /rustc/c8dfcfe046a7680554bf4eb612bad840e7631c4b/library/core/src/panicking.rs:50:5
   3: core::ops::function::FnOnce::call_once
   4: 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.

The floating point comparison failure is expected (the closest representable value is 0x205481A9).

According to parse_float() documentation, it says the code can panic if there is too many digits... It also says that:

It is therefore up to the caller to validate this input.

Isn't that too dangerous? I mean, I'm not controlling the value being sent to nom::number::complete::float() so I cannot validate anything. And in addition, I am not sure what kind of validation is required.

In an ideal world, minimal_lexical::parse::parse_float::parse_float() would not panic for any input...

[Stargateur]

could we have a BNF for the function to respect ? similar to what f64::from_str define:

Float  ::= Sign? ( 'inf' | 'NaN' | Number )
Number ::= ( Digit+ |
             Digit+ '.' Digit* |
             Digit* '.' Digit+ ) Exp?
Exp    ::= [eE] Sign? Digit+
Sign   ::= [+-]
Digit  ::= [0-9]