Skip to content
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

CVE-2020-26235 advisory for time 0.1 dependency #602

Closed
acim opened this issue Oct 11, 2021 · 33 comments · Fixed by tag1consulting/goose#559
Closed

CVE-2020-26235 advisory for time 0.1 dependency #602

acim opened this issue Oct 11, 2021 · 33 comments · Fixed by tag1consulting/goose#559

Comments

@acim
Copy link

acim commented Oct 11, 2021

Please update time dependency.

Inverse Dependency graph
time 0.1.44 (registry+https://github.com/rust-lang/crates.io-index)
└── chrono 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)

@timvisee
Copy link

timvisee commented Oct 11, 2021

Yes!

I've prepared a PR for it some time ago, but it hasn't been merged yet: #578

@CryZe
Copy link
Contributor

CryZe commented Oct 11, 2021

Everything below 0.2.7 is unaffected:
https://github.com/rustsec/advisory-db/blob/main/crates/time/RUSTSEC-2020-0071.md?plain=1#L36

However chrono does seem somewhat unmaintained, so maybe it makes sense to just switch to time 0.3 as your dependency entirely, which covers a lot of chrono's surface area, while also reducing the dependency count.

@acim
Copy link
Author

acim commented Oct 11, 2021

Many projects are using chrono, they all have to drop chrono and use time 0.3, as you suggested.

 time 0.1.44 (registry+https://github.com/rust-lang/crates.io-index)
└── chrono 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)
    └── mysql_common 0.27.5 (registry+https://github.com/rust-lang/crates.io-index)
        └── mysql_async 0.28.1 (registry+https://github.com/rust-lang/crates.io-index)
time 0.1.44 (registry+https://github.com/rust-lang/crates.io-index)
├── chrono 0.4.19 (registry+https://github.com/rust-lang/crates.io-index)
│   ├── json_env_logger 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)
│   │   └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   ├── k8s-openapi 0.13.1 (registry+https://github.com/rust-lang/crates.io-index)
│   │   ├── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   ├── kube 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   │   ├── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   │   └── kube-runtime 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   │       └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   ├── kube-core 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   │   └── kube 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   │       ├── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   │       └── kube-runtime 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   │           └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   └── kube-runtime 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │       └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   ├── kube 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │   ├── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   │   └── kube-runtime 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│   │       └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│   └── kube-core 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│       └── kube 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│           ├── bond 0.1.5 (path+file:///home/runner/work/bond/bond)
│           └── kube-runtime 0.61.0 (registry+https://github.com/rust-lang/crates.io-index)
│               └── bond 0.1.5 (path+file:///home/runner/work/bond/bond)

@Arnavion
Copy link

Arnavion commented Oct 11, 2021

Actually, this conversation is founded on a wrong assumption. chrono has the issue not because of its time 0.1 dependency, but because it directly calls localtime_r in the same way that time 0.1 does. AFAICT the code from time 0.1 that calls localtime_r is never called by chrono, at least not in the Local::now call path.

So unless I'm missing something, while it's unfortunate that the PR to update to time 0.3 hasn't been merged for a long time, it also won't actually fix the crash reported here.

@legoktm
Copy link

legoktm commented Oct 18, 2021

@Arnavion wrote:

Actually, this conversation is founded on a wrong assumption. chrono has the issue not because of its time 0.1 dependency, but because it directly calls localtime_r in the same way that time 0.1 does. AFAICT the code from time 0.1 that calls localtime_r is never called by chrono, at least not in the Local::now call path.

Should a separate security advisory be issued for chrono then, in addition to the one for time that transitively affects chrono?

@CryZe
Copy link
Contributor

CryZe commented Oct 18, 2021

Yeah definitely.

@tarcieri
Copy link

I've opened a PR to add an advisory for chrono here: rustsec/advisory-db#1082

@timvisee
Copy link

timvisee commented Oct 18, 2021

Actually, this conversation is founded on a wrong assumption. chrono has the issue not because of its time 0.1 dependency, but because it directly calls localtime_r in the same way that time 0.1 does. AFAICT the code from time 0.1 that calls localtime_r is never called by chrono, at least not in the Local::now call path.

So unless I'm missing something, while it's unfortunate that the PR to update to time 0.3 hasn't been merged for a long time, it also won't actually fix the crash reported here.

After reading through the issues: would using a shared lock for set_var and the localtime_r invocation fix this issue?

If so, I'd be happy to try and implement it. In the hopes of it getting merged of course.

@tarcieri
Copy link

Per the discussion on time#293 it sounds like the best solution would be to reimplement localtime_r in pure Rust, possibly translating the implementation from Musl.

@jnicholls
Copy link

Per the discussion on time#293 it sounds like the best solution would be to reimplement localtime_r in pure Rust, possibly translating the implementation from Musl.

Yep I think that would be the proper thing to do longterm. And the less explicit dependency on libc, the better.

ISibboI added a commit to ISibboI/vocabulary-learning-application that referenced this issue Aug 12, 2023
It is about some old version of the time crate used by chrono,
but the chrono authors say that they don't actually use the
affected functions.

See chronotope/chrono#602 (comment)
@djc
Copy link
Contributor

djc commented Sep 7, 2023

FWIW, we've released chrono 0.4.30 without the time 0.1 dependency. See the release notes for more context.

primeos-work added a commit to primeos-work/butido that referenced this issue Sep 11, 2023
Getting rid of the old time 0.1 dependency is desirable as it isn't
maintained anymore and triggers security alerts due CVE-2020-26235 [0].

The chrono crate finally dropped the dependency on time in version
0.4.30 [1].

[0]: https://github.com/science-computing/butido/security/dependabot/4
[1]: chronotope/chrono#602 (comment)

Signed-off-by: Michael Weiss <michael.weiss@atos.net>
karlpvoss added a commit to karlpvoss/seella that referenced this issue Sep 23, 2023
Update and lock patch versions for all dependencies.

Addresses: chronotope/chrono#602
karlpvoss added a commit to karlpvoss/seella that referenced this issue Sep 23, 2023
Update and lock patch versions for all dependencies.

Addresses: chronotope/chrono#602
@djc djc unpinned this issue Jan 24, 2024
ammernico pushed a commit to ammernico/butido that referenced this issue Apr 30, 2024
Getting rid of the old time 0.1 dependency is desirable as it isn't
maintained anymore and triggers security alerts due CVE-2020-26235 [0].

The chrono crate finally dropped the dependency on time in version
0.4.30 [1].

[0]: https://github.com/science-computing/butido/security/dependabot/4
[1]: chronotope/chrono#602 (comment)

Signed-off-by: Michael Weiss <michael.weiss@atos.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.