Insecure dependencies: chrono/time

[itamarst]

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]

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

[abonander]

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]

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]

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

[abonander]

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():

• Postgres:

  sqlx/sqlx-core/src/postgres/types/chrono/datetime.rs

  Line 95 in 12d9f54

  Ok(Local.from_utc_datetime(&naive))

• MySQL:

  sqlx/sqlx-core/src/mysql/types/chrono.rs

  Line 60 in 8bcac03

  Ok(<DateTime<Utc> as Decode<'r, MySql>>::decode(value)?.with_timezone(&Local))

  • DateTime::with_timezone(&tz) invokes tz.from_utc_datetime(): https://docs.rs/chrono/latest/src/chrono/datetime.rs.html#212-214
• SQLite:

  sqlx/sqlx-core/src/sqlite/types/chrono.rs

  Line 91 in 8561891

  Ok(Local.from_utc_datetime(&decode_datetime(value)?.naive_utc()))

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

• chrono::sys::Timespec::local(): https://github.com/chronotope/chrono/blob/3467172c31188006147585f6ed3727629d642fed/src/offset/local.rs#L188
• chrono::sys::inner(unix.rs)::time_to_local_tm(): https://github.com/chronotope/chrono/blob/3467172c31188006147585f6ed3727629d642fed/src/sys.rs#L64
• libc::localtime_r(): https://github.com/chronotope/chrono/blob/f6bd567bb677262645c1fc3131c8c1071cd77ec3/src/sys/unix.rs#L84

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.