vulnerabilities in unmaintained chrono dependency

[ghost]

chrono is unmaintained and not willing to fix discovered vulns in its dependencies. this results in cargo audit findings for projects using rusqlite. other crates have been swapping to using time directly. can the same be done here to avoid findings and security issues?

https://rustsec.org/advisories/RUSTSEC-2020-0159.html
https://rustsec.org/advisories/RUSTSEC-2020-0071.html

[thomcc]

Yeah, I mean, we don't do anything with the vulnerable code, and we support it only so that people can read datetimes and the like out of their databases. I'm in favor of removing it though, we already support time.

[gwenn]

chrono is unmaintained

https://github.com/chronotope/chrono/commits/main
chronotope/chrono#650
Are you sure ?

[ghost]

chronotope/chrono#639 (comment) seems as though the owner is mia and blocking things. being on rust 2015 doesnt help either. either way, there is a medium, almost high, vuln in the version used by rusqlite.

[gwenn]

Quoting @thomcc:

we don't do anything with the vulnerable code

...

[thomcc]

I'll look into it and see if there's anything we can do here, but ultimately a lot of code uses chrono and rusqlite in order to access to time information which is stored in the database. That code is largely stuck without a migration path in this situation, which is... unfortunate.

It is also true that rusqlite's usage is not impacted and does not call vulnerable APIs are not used. Sadly, the existing tools can't verify this as far as I know¹.

However, if your rusqlite dependency does not have features = ["chrono"], then we will not depend on chrono at all, and so it will not be present in your dependency graph. The tools I'm aware of (both cargo audit and cargo deny) will not report an issue with rusqlite in this case. Please let me know if this is not the case, as it would change my feelings about how important this is to address.

Anyway, I've been in favor of removing this for a while, but this is the sort of thing that @gwenn and I need to agree on. At the very least, they feel more strongly about it than I do.

(My reason for wanting it gone is so that if a new vulnerability is found that impacts an API we do use, we don't accidentally miss this fact, and also I'd like to add cargo audit into our CI)

Footnotes

1. Even though the advisory database contain a list of vulnerable functions and such, no tool exists that uses that information. This is because doing this requires performing trait resolution and call graph analysis, and in practice this requires either writing a non-trivial part of a Rust compiler, or direct access to rustc's internals. ↩

[thomcc]

chronotope/chrono#639 (comment) seems as though the owner is mia and blocking things. being on rust 2015 doesnt help either. either way, there is a medium, almost high, vuln in the version used by rusqlite.

Worth noting that (as of #1031) we avoid the vulnerability addressed by that PR, although I believe there's other issues inside chrono that apply even without the old version of time. (Note that even prior to #1031, we still weren't using a vulnerable API, but were just pulling in a version of the time crate which had the vuln)

It is worth noting that largely speaking the origin of this vulnerability is in the Rust standard library rather than chrono. std::env::set_var should not be a safe function, as it is an API which becomes unsound in the presence of non-rust code that accesses the environment.

Quite a bit has been written about this in https://internals.rust-lang.org/t/synchronized-ffi-access-to-posix-environment-variable-functions/15475, and the outcome is likely that set_var will be deprecated and the replacement will be unsafe.

[gwenn]

https://www.reddit.com/r/rust/comments/tqzeek/comment/i2ku631/?utm_source=share&utm_medium=web2x&context=3

[gwenn]

https://www.reddit.com/r/rust/comments/w8m8lx/chrono_0420rc1_has_just_been_released/

[gwenn]

https://www.reddit.com/r/rust/comments/wg3fks/chrono_0420_has_been_released_fixing_the/
Would you mind closing the issue ?
Thanks.