rust-pcap / pcap

Rust language pcap library
Apache License 2.0
617 stars 142 forks source link

Libloading declared most of its API unsafe in a subsequent release #345

Closed musicinmybrain closed 5 months ago

musicinmybrain commented 5 months ago

This project depends on libloading = 0.6:

https://github.com/rust-pcap/pcap/blob/8b7bcfe12c79955c228e5657a1e63cdcce0af3f5/Cargo.toml#L43

It’s used in exactly one place:

https://github.com/rust-pcap/pcap/blob/8b7bcfe12c79955c228e5657a1e63cdcce0af3f5/build.rs#L102

But in the subsequent 0.7.0 release, libloading realized that most of its API, including Library::new, should have been unsafe. (The current release is 0.8.3)

“Hiding” soundness issues by using an old libloading version would seem to have a limited impact since only build.rs is affected, but it would be better to upgrade to a current version and actually deal with the unsafe API.

Stargateur commented 5 months ago

That really not important, the only diff would be put the unsafe and close your eye cause we would have no way of ensuring the lib is not doing some static thing that we can't control.

musicinmybrain commented 5 months ago

That really not important, the only diff would be put the unsafe and close your eye cause we would have no way of ensuring the lib is not doing some static thing that we can't control.

Would you consider a PR that did simply that? Update the dependency to 0.8 and wrap the libloading::Library::new(libfile) file in unsafe { }? That would acknowledge the situation and avoid the need to use an old libloading version indefinitely – both small but real improvements, I think.

Wojtek242 commented 5 months ago

An update would have to happen eventually. A PR would be welcome. Please note that the current pipeline is failing due to some weird Windows issue and some new clippy lints, but outside of that things should pass.