Clarify advisories about crates that read from (not write to) the environment

[briansmith]

There are some advisories like https://rustsec.org/advisories/RUSTSEC-2020-0159 and https://rustsec.org/advisories/RUSTSEC-2020-0071 that have spawned good discussion and are (probably) leading to good changes to the standard library. However, these advisories also seem to be at odds with the current thinking, which is that it is OK to read from the environment but not write to the environment.

I think these advisories are now distracting from real soundness (memory safety and other race condition) issues like chronotope/chrono#648 that are the result of writing to the environment.

We're in a weird state where we now have issues like #926 that stay open here because we can't agree on whether reading from the environment constitutes a vulnerability. I don't think we can really consider it to be a vulnerability at this point, so I think those advisories should be closed with such an explanation. Are we waiting for the Rust library team to agree and to do the deprecation of env::{set_var,remove_var}?

[briansmith]

Oh, sorry, I forgot the main request I had: We should update the text of the RUSTSEC advisories and the related CVE with more explanation of the issue, as the current advisory text, though not totally wrong, not really helpful in helping people understand the issue.

Otherwise, presumably there are many, many RUSTSEC advisories to file against many, many crates, for the same reason.

[tarcieri]

Yeah, this was something we've waffled on a few times as the individual advisories were part of a much bigger systemic issue that goes all the way down to the POSIX API. We had previously rejected some advisories for this and ended up accepting the ones for time and chrono (the latter of which I filed due to the amount of noise people were making), perhaps in haste.

I agree they're not particularly helpful in terms of describing the problem or what action should be taken, even if that's just adding the advisory to audit.toml's ignore list.

In the time since I think there's been a lot of discussion, perhaps in part motivated by the amount of noise this created, and perhaps a clearer path to a solution.

I think at the very least these advisories aren't security critical and can thus be demoted to informational = "unsound" which in turn demotes them to warnings in cargo audit.

We could also consider yanking them, especially if env::set_var is deprecated.

[pinkforest]

Sum summarum:

There are upstream rust-lang/rust issue(s) rust-lang/rust#90308 and libs are well aware of this by now with extensive discussions.

time was only re-filed because the maintainer wanted it back - #1242 (comment)

chrono was only re-filed to advertise fix because there was RiR localtime_r in 0.4.20 - #1310

Filing something against something where the maintainer disagrees that this should be done has made these controversial downstream in addition that there can be no fix in crate itself.

I think our current established policy covers this now - generally if the maintainer agrees we can file and / or there is a fix but there is no straightforward yes / no answer for this as these are controversial - and I believe it would be futile to try to cover this in our docs I think.

Closing as completed since we now have the precedent on this -

Re-open if we need to action any documentation / such fix.

Thanks all 👍