paupino / rust-decimal

Decimal number implementation written in pure Rust suitable for financial and fixed-precision calculations.
https://docs.rs/rust_decimal/
MIT License
1.03k stars 183 forks source link

Update borsh crate due to a security vulnerability issue. #595

Closed pxp9 closed 1 year ago

pxp9 commented 1 year ago

Dear @paupino this is a awesome crate, I am using it as dependency of mysql-common which is a direct dependency of my project,

This needs to be updated to a borsh version > 0.10.3 beacuse all the versions below that one has a this vulnerability issue, according to Dependabot Alerts.

image

Thank you for your attention

paupino commented 1 year ago

Since this is a library, I don't actually commit the Cargo.lock so that it will inherit the dependencies of the consuming project. If you run a cargo update in your project then does this go away? As an example, the Cargo.lock within the workspace of the library itself is:

[[package]]
name = "borsh"
version = "0.10.3"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "4114279215a005bc675e386011e594e1d9b800918cea18fcadadcce864a2046b"
dependencies = [
 "borsh-derive",
 "hashbrown 0.13.2",
]

Or are you suggesting that this library should assert >= 0.10.3? Since this library is likely more of a dependent library as opposed to defining library for borsh I'm thinking it should be left more to the parent library?

Edit: also, thank you for raising this to my attention!

pxp9 commented 1 year ago

Dear @paupino, thank you for your fast response.

I think Cargo.toml should define borsh dependency as 0.10 instead as 0.10.0 because I think if you leave as 0.10.0 you do not get patch updates such this security issue, that is solve >= 0.10.3 patch update.

This line

https://github.com/paupino/rust-decimal/blob/9787b1d3095cd8b8d1500a904edc0dffe88e3a9d/Cargo.toml#L28C3-L28C3

I think should be change to this one.

borsh = { default-features = false, optional = true, version = "0.10" }

Edit : You are right cargo update , updates the dependency, I also think that would be good to update Cargo.toml file.

paupino commented 1 year ago

I've dropped the revision from the Cargo.toml to avoid confusion - this will be released with 1.31!

couchand commented 1 year ago

Unfortunately, the fix for borsh is not included in version 0.10.3. The first version with the fix is 1.0.0-alpha.1, so clearing the security alert will require an update the major version of borsh, which will clearly need to wait for a full version rather than just a prerelease.