search

grafana / pyroscope-rs

Pyroscope Profiler for Rust. Profile your Rust applications.
Apache License 2.0
132 stars 22 forks source link

Migrate away from `json` dependency that is unmaintained and unsound #139

Open mati865 opened 6 months ago

mati865 commented 6 months ago

Describe the bug you encountered:

Cargo-deny reports several issues when adding this crate to the tree. One of them is related to json crate that is unmaintained and unsound. It'd be welcome if this crate could migrate to one of alternatives listed in the advisory.

What did you expect to happen instead?

Clean report from cargo-deny.

How did you install pyroscope-rs?

Added it as a dependency to one of the crates in the project.

version: pyroscope 0.5.7 os: Ubuntu 22.04

korniltsev commented 6 months ago

Are there any REAL problems with the library? Are there any bugs or security issues with it?

An alternative wold be to get rid of the json library alltogether. It is currently used to pass some data withing same process from one piece of code to another, which I am not very proud of.

korniltsev commented 6 months ago

Pull requests are welcome ;)

mati865 commented 5 months ago

Are there any REAL problems with the library?

It contains undefined behaviour if as_bytes is called before attach as the report mentions: https://github.com/maciejhirsz/json-rust/issues/196 Since there is no visible segfault right now I assume the struct is initialized properly but that sounds a bit fragile. So probably the only issue right now is having to explain why it's ok to ignore this specific advisory.

https://rustsec.org/advisories/RUSTSEC-2022-0081 lists some alternatives, jzon particularly looks like drop in replacement.

An alternative wold be to get rid of the json library alltogether. It is currently used to pass some data withing same process from one piece of code to another, which I am not very proud of.

Sounds like a viable solution as well, I'll try to look into it in the near future.