-
-
Notifications
You must be signed in to change notification settings - Fork 295
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
chrono not maintaned - CVE-2020-26235 in time #650
Comments
Thanks for the report. Yeah, we will look into that. We don't have a lot of usage of Usage of
|
The linked comment:
It's "somewhat" unmaintained. I'd ask Also, it says versions below |
Yeah, that's true. If we were affected by the CVE |
That would be the best, let's hope you have success with this. |
cargo audit doesn't fail, but cargo pants does:
|
Note that |
That might be a bug in |
FYI: chronotope/chrono#499 (comment) + time-rs/time#293 (comment) Edit: And also chronotope/chrono#602 (comment) |
Here's my thoughts from k8s-openapi's POV:
So I'm not too worried about switching unless other parts of the ecosystem are planning to. |
Yeah, it would not be an easy thing to get the ecosystem to change. Some comments: It might still be "right" to update the CVE to match old versions of That said, migrating can be done for us. Even the prevalent Anyway, I left a comment on the chrono PR in the meantime to ask if people are watching it (even if it may or may not be as urgent as I originally thought). |
The Lack of features is not a reason to stick with |
Actually, I take that back. Every struct in Edit: Okay, looks like I'm expected to use |
Maintainer of time here. Happy to answer any questions you may have. Regarding usage: time 0.3 has minimal download count because it was released recently. Time 0.2 had significant uptake. Time (especially 0.3) is far more lightweight. The 0.3 series has zero mandatory dependencies. Regarding formatting: Is the format known at compile-time? If so, use the macro. If not, you can still use I can't speak as to chrono, as I only know what others can see as well. However, there hasn't been a commit since January. That's a pretty significant amount of time. |
No worries. I've already implemented it. I don't have any questions.
Yes, I did consider this, since I'm not a fan of the macro DSL. But non-exhaustiveness also prevents It's not a big deal either way. I had to write it once and be done with it. |
For what it's worth I use a macro internally (for testing purposes) that expands struct literal syntax into the latter of what you said. It's absolutely a hack but 100% worth it. I wish Rust had a way to permit |
So far as getting rid of time 0.1 is concerned, isn't the simplest thing to disable the eg in k8s-openapi, this seems to be sufficient: diff --git a/Cargo.toml b/Cargo.toml
index dfe517b1a..4663aa37d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -30,7 +30,7 @@ links = "k8s-openapi-0.13.1"
[dependencies]
base64 = "0.13"
bytes = "1"
-chrono = { version = "0.4.1", features = ["serde"] }
+chrono = { version = "0.4.1", default-features = false, features = ["alloc", "serde"] }
http = { version = "0.2", optional = true }
percent-encoding = { version = "2", optional = true }
schemars = { version = "0.8", optional = true } I realise that a security advisory for chrono is going to follow which may be harder to deal with, especially if chrono maintainers continue to be inactive, but this is surely a step in the right direction. |
Now cargo audit also shows the error, so this will break so many builds. |
For the sake of shutting up any auditing tools, yes. For the sake of actually solving the problem that triggered the CVE in the first place, no. |
Yes; as as I say, I understand that fixing the upcoming duplicate CVE in chrono is likely to be a tougher problem. Still, baby steps... |
I can do that (and Edit: Done in Arnavion/k8s-openapi@1a14517 |
This also disables the extra-traits feature of syn, since it was being used for one `#[derive(Debug)]` that can be done without, and one `!=` involving `AttrStyle` that can be done with `if let` instead. base64, chrono and serde_json have "alloc" and "std" features where one of them is required. In all three cases, it's possible to have both features enabled and that's essentially treated the same as if only the std feature is enabled. So this commit enables the "alloc" feature, and if anything else in the end user's dep tree enables the "std" feature, that's fine. The one noticeable breaking change is that enabling the schemars feature of k8s-openapi no longer enables the derive feature of schemars, so users who want to use `#[derive(schemars::JsonSchema)]` in their own code will need to add an explicit dependency on schemars to enable that feature. Ref: kube-rs/kube#650 (comment)
This also disables the extra-traits feature of syn, since it was being used for one `#[derive(Debug)]` that can be done without, and one `!=` involving `AttrStyle` that can be done with `if let` instead. base64, chrono and serde_json have "alloc" and "std" features where one of them is required. In all three cases, it's possible to have both features enabled and that's essentially treated the same as if only the std feature is enabled. So this commit enables the "alloc" feature, and if anything else in the end user's dep tree enables the "std" feature, that's fine. The one noticeable breaking change is that enabling the schemars feature of k8s-openapi no longer enables the derive feature of schemars, so users who want to use `#[derive(schemars::JsonSchema)]` in their own code will need to add an explicit dependency on schemars to enable that feature. Ref: kube-rs/kube#650 (comment)
Closing this as chrono 0.4.20 is out. |
crono depends on vulnerable version of time and it seems not to be well maintained. Could you maybe replace it completely with some newer version of time as suggested here.
The text was updated successfully, but these errors were encountered: