Open ydongyeon opened 2 weeks ago
I think this should be changed to use first(&self) -> Option<u8>
and move the call of is_empty
in parse_float
to parse_number
to guarantee local safety invariants.
Thank you for the quick response! I agree that if you apply the patch as you suggested, it will indeed help ensure the local safety invariants. I appreciate your insight and look forward to the update.
This is patched in fast-float2 as of the latest commits, and soon to be in release 0.2.2
:
https://github.com/Alexhuszagh/fast-float-rust/pull/7
Summary
An unsafe memory access vulnerability in the
first
method of theAsciiStr
struct allows arbitrary memory access when empty buffer is provided, potentially triggering undefined behavior.Details
Hi,
First, I want to extend my gratitude for maintaining this excellent crate. I’ve identified a potential security vulnerability: Null Pointer Dereference.
Environment:
Steps to reproduce:
(1) Replace fast-float-rust/src/common.rs with the modified common.rs file as below.
(2) Run the test using the ASan flag.
Details: // fast-float-rust/src/common.rs
In this case, the
first
method within theAsciiStr
struct uses theunsafe
keyword to access memory without performing bounds checking. Specifically, it directly dereferences a pointer offset byself.ptr
. This approach violates Rust’s memory safety guarantees, as it can lead to invalid memory access if empty buffer is provided.Actual results : running 1 test AddressSanitizer:DEADLYSIGNAL ==287389==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffff41ff0e1 at pc 0x555555fb451b bp 0x7ffff4cfe390 sp 0x7ffff4cfe388 READ of size 1 at 0x7ffff41ff0e1 thread T1
...
running 1 test AddressSanitizer:DEADLYSIGNAL ==1314890==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000001 (pc 0x555555686ff8 bp 0x7ffff4afe500 sp 0x7ffff4afe430 T1) ==1314890==The signal is caused by a READ memory access. ==1314890==Hint: address points to the zero page.
0 0x555555686ff8 in fast_float::common::AsciiStr::first::h7c7d95f37c6a3fac /home/dy3199/Fuzzing-Test/fast-float-rust/src/common.rs:39:18
==1314890==Register values: rax = 0x0000000000000001 rbx = 0x00007ffff4afe460 rcx = 0x0000000000000001 rdx = 0x0000000000000001
rdi = 0x00000ffffe7bfe04 rsi = 0x0000000000000000 rbp = 0x00007ffff4afe500 rsp = 0x00007ffff4afe430
r8 = 0x0000000000000000 r9 = 0x00007fffffffff01 r10 = 0x00007fffffffff01 r11 = 0x142ad95dd2047d01
r12 = 0x0000000000000000 r13 = 0x00007ffff4afecf0 r14 = 0x000000003b9aca00 r15 = 0x7fffffffffffffff
AddressSanitizer can not provide additional info. SUMMARY: AddressSanitizer: SEGV /home/dy3199/Fuzzing-Test/fast-float-rust/src/common.rs:39:18 in fast_float::common::AsciiStr::first::h7c7d95f37c6a3fac Thread T1 created by T0 here:
0 0x55555563ebf1 in pthread_create /rustc/llvm/src/llvm-project/compiler-rt/lib/asan/asan_interceptors.cpp:250:3
==1314890==ABORTING
Recommended Patch: Given the potential memory safety issues, I would suggest: Implementing input validation in
first
to safely handle unexpected empty struct.Impact
The
first
method in theAsciiStr
struct of thefast-float-rust
library introduces an unsafe vulnerability by allowing arbitrary memory access without bounds checking. This flaw can lead to undefined behavior. Although exploiting this vulnerability may be challenging, it undermines Rust’s core memory safety guarantees.Although these bugs may be difficult to exploit in practice, I understand that the Rust community reports such issues to RUSTSEC in an effort to further enhance memory safety. Rust considers these issues critical to memory safety, regardless of whether they have been exploited, and takes proactive measures to either fix or report them. I have included references to similar cases below for your consideration.
Panic on overflow in subtraction RUSTSEC-2023-0078 RUSTSEC-2022-0078 RUSTSEC-2022-0012 [Potential unsoundnesses (not yet determined) with use of unsafe · Issue #37 · aldanor/fast-float-rust](https://github.com/aldanor/fast-float-rust/issues/37) Fix Undefined Behavior in
check_len
· Issue #28 · aldanor/fast-float-rust