Security issues found on dep for kvdb-rocksdb (owning_ref = "0.4.0")

[eduardonunesp]

The issue is described on https://rustsec.org/advisories/RUSTSEC-2022-0040 and https://github.com/noamtashma/owning-ref-unsoundness, it was catch by the cargo-deny checker and affects the kv-rocksdb

$ cargo deny check advisories

2022-08-09 20:21:59 [WARN] unable to find a config path, falling back to default config
error[A001]: Multiple soundness issues in `owning_ref`
   ┌─ /tmp/parity-common/Cargo.lock:91:1
   │
91 │ owning_ref 0.4.1 registry+https://github.com/rust-lang/crates.io-index
   │ ---------------------------------------------------------------------- security vulnerability detected
   │
   = ID: RUSTSEC-2022-0040
   = Advisory: https://rustsec.org/advisories/RUSTSEC-2022-0040
   = - `OwningRef::map_with_owner` is [unsound](https://github.com/Kimundi/owning-ref-rs/issues/77) and may result in a use-after-free.
     - `OwningRef::map` is [unsound](https://github.com/Kimundi/owning-ref-rs/issues/71) and may result in a use-after-free.
     - `OwningRefMut::as_owner` and `OwningRefMut::as_owner_mut` are [unsound](https://github.com/Kimundi/owning-ref-rs/issues/61) and may result in a use-after-free.
     - The crate [violates Rust's aliasing rules](https://github.com/Kimundi/owning-ref-rs/issues/49), which may cause miscompilations on recent compilers that emit the LLVM `noalias` attribute.
     
     No patched versions are available at this time. While a pull request with some fixes is outstanding, the maintainer appears to be unresponsive.
   = Announcement: https://github.com/noamtashma/owning-ref-unsoundness
   = Solution: No safe upgrade is available!
   = owning_ref v0.4.1
     └── kvdb-rocksdb v0.15.2

[ordian]

We're not using any of these methods.

That being said, a refactor to get rid of owning_ref is very welcome. Perhaps by implementing #437 before we deprecate kvdb-rocksdb.

[insipx]

@ordian is kvdb-rocksdb being replaced with another crate? why the deprecation

[ordian]

@insipx rocksdb will be deprecated in favor for parity-db eventually, I can't tell you the timeline for that though.

[eduardonunesp]

Thanks @ordian, I going to close this thread, feel free to reopen if needed.

[TheArchitect108]

@ordian from serai-dex/serai#81

#437 was closed 6 months ago as wontfix due to how rocksdb would be "deprecated" for paritydb, despite there still being no timeline on when and a lack of further reasoning directly stated (which would've been appreciated, albeit may just be available elsewhere and accordingly fine).

kvdb-rocksdb also uses rocksdb 0.18, which has a vulnerability in a certain API. While that may not be relevant, I'd appreciate using 0.19 to close out the advisory.

Could you at least bump or accept a PR to bump rocksdb to 0.19 given the lack of timeline?

[kayabaNerve]

I'd request either both of the above issues are fixed or this library is actually marked as deprecated. Marking wontfix on security issues due to planned deprecation is effective deprecation. Parity needs to take ownership of this library or acknowledge that.

[ordian]

I already mentioned this issue doesn't apply to kvdb-rocksdb. There's nothing to fix here as a security vulnerability. Having a dependency that is unmaintained is not a security issue.

[ordian]

Could you at least bump or accept a PR to bump rocksdb to 0.19 given the lack of timeline?

Yes, sure. And just to be clear, kvdb-rocksdb is not unmaintained or deprecated as of yet. PRs are always welcome :)

[vstakhov]

I will make a PR later today, as I have done such an upgrade quite recently.

[ordian]

I've got an idea how to remove owning_ref, will submit a PR soon.

[ordian]

Both issues are addressed by #662 and #661 respectively.