Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lexical dependency considered “unsound” #3

Open
backspace opened this issue Apr 16, 2024 · 4 comments
Open

Lexical dependency considered “unsound” #3

backspace opened this issue Apr 16, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@backspace
Copy link

Hey, thanks for your work on this, I used it in an Axum application where I was receiving duplicate query parameters and your library could handle it, unlike the built-in extractor.

I’ve been getting security audit errors about the dependency on lexical:

error[unsound]: Multiple soundness issues
    ┌─ /home/runner/work/adventures/adventures/unmnemonic_devices_vrs/Cargo.lock:114:1
    │
114 │ lexical 6.1.1 registry+https://github.com/rust-lang/crates.io-index
    │ ------------------------------------------------------------------- unsound advisory detected
    │
    = ID: RUSTSEC-2023-0055
    = Advisory: https://rustsec.org/advisories/RUSTSEC-2023-0055
    = `lexical` contains multiple soundness issues:
      
       1. [Bytes::read() allows creating instances of types with invalid bit patterns](https://github.com/Alexhuszagh/rust-lexical/issues/102)
       1. [BytesIter::read() advances iterators out of bounds](https://github.com/Alexhuszagh/rust-lexical/issues/101)
       1. [The `BytesIter` trait has safety invariants but is public and not marked `unsafe`](https://github.com/Alexhuszagh/rust-lexical/issues/104)
       1. [`write_float()` calls `MaybeUninit::assume_init()` on uninitialized data, which is is not allowed by the Rust abstract machine](https://github.com/Alexhuszagh/rust-lexical/issues/95)
      
      The crate also has some correctness issues and appears to be unmaintained.
      
      ## Alternatives
      
      For quickly parsing floating-point numbers third-party crates are no longer needed. A fast float parsing algorith by the author of `lexical` has been [merged](https://github.com/rust-lang/rust/pull/86761) into libcore.
      
      For quickly parsing integers, consider `atoi` and `btoi` crates (100% safe code). `atoi_radix10` provides even faster parsing, but only with `-C target-cpu=native`, and at the cost of some `unsafe`.
      
      For formatting integers in a `#[no_std]` context consider the [`numtoa`](https://crates.io/crates/numtoa) crate.
      
      For working with big numbers consider `num-bigint` and `num-traits`.

I’m pretty new to Rust and not that confident about attempting the suggested alternatives, but I could give it a try if you don’t have capacity. I assume this would be a breaking change 🤔

@pooyamb
Copy link
Owner

pooyamb commented May 6, 2024

@backspace Not sure how I missed this issue, thanks for bringing it into my attention.
I will check this out to see if we are affected by the bug or not, and will release a new version with updated dependencies.
Regarding it being a breaking change, I don't believe so. lexical is not directly exposed to the end user and is only used internally, so it should be safe to upgrade or replace.

@pooyamb pooyamb added the bug Something isn't working label May 6, 2024
@pooyamb
Copy link
Owner

pooyamb commented May 7, 2024

@backspace I took a look into it today, and it looks like we might not be affected by the bug. But I think it would still make sense to dump lexical, since it looks unmaintained.
Replacing integer parsing with atoi was straight forward, and I've already implemented that, replacing float parsing on the other hand, was not.
Rust doesn't provide direct ascii &[u8] to f32|f64 parsing and we will need to perform utf-8 checking and get a str first. I believe that would not be wise to do, since float parsing does not require such implications to work, at least as far as I know. Do you have any idea how we can get around that, considering we don't actually know if the provided bytes slice is valid utf-8?

@backspace
Copy link
Author

Do you have any idea how we can get around that

I’m sorry, I don’t! I’m too new to Rust to be of help 😞

@pooyamb
Copy link
Owner

pooyamb commented Jun 20, 2024

@backspace Thanks a lot for the report, that was more than enough buddy, no worries. I pushed a commit today and will hopefully release it soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants