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

Update time dependency in order to mitigate CVE-2020-26235 #632

Closed
wants to merge 2 commits into from

Conversation

jmcph4
Copy link

@jmcph4 jmcph4 commented Dec 7, 2021

Fixes #629.

Thanks for contributing to chrono!

  • Have you added yourself and the change to the changelog? (Don't worry
    about adding the PR number)
  • If this pull request fixes a bug, does it add a test that verifies that
    we can't reintroduce it?

This commit bumps the version of `time` depended on to 0.2.23 in order
to mitigate CVE-2020-26235.
This commit adds the relevant entry to the CHANGELOG.
@Milo123459
Copy link
Member

I'm working on a PR for this right now purely to combine the best of both PRs: the end aim is just to fix the vulnerability without removing functionality.

@jodh-intel
Copy link

@jmcph4, @Milo123459 - Thanks for looking into this! Do you have a feel for when one of these fixes might land? This security PR has been open for one month now which seems like a long time.

Since chrono is a relatively low-level crate, there are many other crates that depend on it. https://crates.io/crates/chrono/reverse_dependencies shows me that > 4,600 other crates use chrono. But all those crates are potentially vulnerable until we get a fix landed for this CVE along with a new chrono release.

bors bot added a commit to meilisearch/milli that referenced this pull request Feb 15, 2022
450: Get rid of chrono in favor of time r=Kerollmops a=irevoire

We only use `chrono` as a wrapper around `time`, and since there has been an [open CVE on `chrono` for at least 3 months now](chronotope/chrono#632) and the repo seems to be [struggling with maintenance](chronotope/chrono#639), I think we should use `time` directly which is way more active and sufficient for our use case.

EDIT: Actually the CVE status has been known for more than 6 months: chronotope/chrono#602

Co-authored-by: Irevoire <tamo@meilisearch.com>
@djc
Copy link
Member

djc commented Mar 6, 2022

This is being worked on in #639.

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 this pull request may close these issues.

Note: Time dependency is outdated
4 participants