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

Insecure dependencies: chrono/time #1586

Closed
itamarst opened this issue Dec 15, 2021 · 5 comments
Closed

Insecure dependencies: chrono/time #1586

itamarst opened this issue Dec 15, 2021 · 5 comments

Comments

@itamarst
Copy link

itamarst commented Dec 15, 2021

Per https://rustsec.org/advisories/RUSTSEC-2020-0071, time versions < 0.2.23 should not be used.

The version of chrono used (0.4.19) depends on a too-old version of time.

@itamarst
Copy link
Author

itamarst commented Dec 15, 2021

Apparently (some?) chrono APIs are also an issue: chronotope/chrono#499 (https://rustsec.org/advisories/RUSTSEC-2020-0159)

@abonander
Copy link
Collaborator

abonander commented Dec 15, 2021

chrono includes time 0.1 as an optional feature that we don't enable.

The versions listed in our Cargo.tomls aren't a hard requirement and can be automatically upgraded in dependency resolution according to the version specification in the leaf project.

It wouldn't hurt to run cargo upgrade for this project* if someone wants to do that, but the onus is mainly on consumers of this crate to run it for their projects.

@jplatte
Copy link
Contributor

jplatte commented Dec 16, 2021

cargo update won't help; there is no patched version of chrono. RUSTSEC-2020-0159 is really the relevant issue and there is no fix for it currently.

The true problem is that std contains a safe function for setting environment variables that calls setenv on unix-like platforms while holding a Rust-specific lock, but this lock can not be obtained separately so there is no safe way to call C functions that might concurrently use getenv which in theory is (almost?) anything in libc and in practice is a bunch of things that you really want to have access to such as localtime_r.

There is a pending PR to deprecate std::env::set_env at rust-lang/rust#90830, and there's some more details in rust-lang/rust#90308 plus the other issues linked there.

@itamarst
Copy link
Author

In the short term, would be useful to document whether or not (and when) sqlx is using the currently-problematic APIs, maybe?

@abonander
Copy link
Collaborator

It looks like the one place where localtime_r() may be invoked indirectly by SQLx is in impl Decode for DateTime<Local> via Local::from_utc_datetime():

Local::from_utc_datetime() calls localtime_r() via this path:

It seems that the general advisory would be to stop using chrono::offset::Local as constructing DateTime<Local> or Date<Local> (which we don't do) in pretty much any manner is what invokes localtime_r().

cargo update won't help;

Not for the chrono crate, no. For the time crate between 0.2.6 and 0.2.23, it will.

@abonander abonander closed this as not planned Won't fix, can't repro, duplicate, stale Jul 1, 2022
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

No branches or pull requests

3 participants