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 the time crate to 0.3 #2884

Closed
sylvestre opened this issue Jan 17, 2022 · 8 comments · Fixed by #3319
Closed

Update the time crate to 0.3 #2884

sylvestre opened this issue Jan 17, 2022 · 8 comments · Fixed by #3319
Labels
good first issue For newcomers!

Comments

@sylvestre
Copy link
Contributor

We are currently using 0.1 and it has some issues.
We should update to version 0.3

Must be updated at least in these crates:

Cargo.toml
src/uu/install/Cargo.toml
src/uu/touch/Cargo.toml
src/uucore/Cargo.toml
@sylvestre sylvestre added the good first issue For newcomers! label Jan 17, 2022
@tertsdiepraam
Copy link
Member

Some interesting context here: time has a vulnerability in version 0.1, see:

I also wonder whether this security issue might be related to

At least it's worth checking whether that last issue is fixed once we have upgraded.

@g-k
Copy link
Contributor

g-k commented Jan 22, 2022

Started looking for migration docs for this. The changelog says versions before 0.2.7 are in release notes and the oldest release with notes is 0.2.0 which refs time-rs/time#190.

@tertsdiepraam
Copy link
Member

So, I also started searching, found their book, and got excited when I found this page: https://time-rs.github.io/book/migration/0.3.html

But it's empty 😄 Looks like we just have to rely on the documentation and tests for this migration.

image

@sylvestre
Copy link
Contributor Author

sylvestre commented Jan 26, 2022

cargo audit returns:

Crate:         time
Version:       0.1.43
Title:         Potential segfault in the time crate
Date:          2020-11-18
ID:            RUSTSEC-2020-0071
URL:           https://rustsec.org/advisories/RUSTSEC-2020-0071
Solution:      Upgrade to >=0.2.23

@g-k
Copy link
Contributor

g-k commented Jan 29, 2022

OK! I started working on this and have fsext.rs in uucore updated.

coreutils uses the time::tm for times and time::Timespec timestamps. Per this comment they were removed in time >0.1.

I'm going to switch the places coreutils relies on those structs to chrono, which is "aims to be a feature-complete superset of the time library" and coreutils already uses elsewhere but it'll likely require some changes to non-test code in touch.

edit: actually that won't fix the CVE since chrono uses a vulnerable version chronotope/chrono#632 so we'll want to get chrono to cut a release (but it looks unmaintained) or target std::time and time 0.3.

edit2: aha! https://time-rs.github.io/format-converter/

@sylvestre
Copy link
Contributor Author

@g-k are you still working on it? thanks

@g-k
Copy link
Contributor

g-k commented Mar 16, 2022

I'd like to finish this but I haven't worked on it recently. Other people should feel free to finish it.

I started working on this in https:/uutils/coreutils/compare/main...g-k:2884-time-0.3?expand=1 running with RUSTFLAGS="--cfg unsound_local_offset". That branch also includes changes to stop using chrono to fix the CVE, but fixing that might be better to fix in a separate PR. The chrono team is working on or fixed the issue, so maybe it's easier to do both now. I also think the unsoundness from the CVE is unlikely to impact utils in this project, since I don't think the utils are multithreaded or run long enough to see changes to the relevant env vars.

@sylvestre
Copy link
Contributor Author

I'd like to finish this but I haven't worked on it recently.

Do you have an eta? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue For newcomers!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants