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

security: Avoid using buggy atty crate #45

Merged
merged 1 commit into from
Aug 26, 2023
Merged

Conversation

Techcable
Copy link
Member

Switches to newer is-terminal crate instead.
This functionality is also availible on the nightly Rust stdlib as a std::io::IsTerminal trait.

Avoids RUSTSEC-2021-0145 (softprops/atty#50)
Fixes slog-rs/slog#319

Based on the information in the vulnerability database, I don't consider this a particularly serious bug.

In practice however, the pointer won't be unaligned
unless a custom global allocator is used.

Switches to newer `is-terminal` crate instead.
This functionality is also availible on the nightly
Rust stdlib as a `std::io::IsTerminal` trait.

Avoids RUSTSEC-2021-0145 (softprops/atty#50)
Fixes slog-rs/slog#319

Based on the information in the vulnerability database,
I don't consider this a particularly serious bug.

> In practice however, the pointer won't be unaligned
   unless a custom global allocator is used.
Copy link

@willbuckner willbuckner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@willbuckner
Copy link

Downloaded once_cell v1.16.0
error: failed to parse manifest at `/home/runner/.cargo/registry/src/github.aaakk.us.kg-1ecc6299db9ec823/once_cell-1.16.0/Cargo.toml`

Caused by:
  feature `edition[20](https://github.com/slog-rs/term/actions/runs/3562324356/jobs/5983962221#step:4:21)[21](https://github.com/slog-rs/term/actions/runs/3562324356/jobs/5983962221#step:4:22)` is required

  this Cargo does not support nightly features, but if you
  switch to nightly channel you can add
  `cargo-features = ["edition2021"]` to enable this feature
Error: Process completed with exit code 101.

@sunfishcode
Copy link

That appears to be an unrelated issue. once_cell, which term already depends on through thread_local, recently updated to use edition = "2021", which is incompatible with Rust 1.53.

You can work around this issue by downgrading once_cell from 1.16.0 to 1.14.0, such as by this command:

cargo update --package once_cell --precise 1.14.0

@Techcable
Copy link
Member Author

That appears to be an unrelated issue. once_cell, which term already depends on through thread_local, recently updated to use edition = "2021", which is incompatible with Rust 1.53.

You can work around this issue by downgrading once_cell from 1.16.0 to 1.14.0, such as by this command:

cargo update --package once_cell --precise 1.14.0

Yes. Unfortunately cargo's dependency resolver doesn't respect Rust MSRV requirements :/

@adamchalmers
Copy link

adamchalmers commented Jul 14, 2023

Hello! Just wondering if @Techcable I can help moving this PR forward. I'd like this PR merged because it's starting to give me GitHub dependabot warnings. Obviously I can just dismiss them (the atty issue cannot actually be exploited in my project) but I also like the slog-term crate, so if you want me to take over this PR and fix the once-cell issues I can do that.

@rkday-pro
Copy link
Contributor

This doesn't quite work even with the once_cell issues fixed - https://github.com/sunfishcode/is-terminal/blob/main/Cargo.toml depends on rustix which depends on bitflags 2.3.3, which also has edition = 2021 (https://github.com/bitflags/bitflags/blob/main/Cargo.toml).

This might need an MSRV bump - is_terminal has an MSRV of 1.63 and that's the lowest bump I could make work.

#47 makes that bump on top of these commits.

@Techcable Techcable merged commit 7f43921 into master Aug 26, 2023
Techcable added a commit that referenced this pull request Aug 26, 2023
Bump MSRV to 1.63

Supersedes my PR #45
@Techcable
Copy link
Member Author

Merged #47, which supersedes this PR.

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

Successfully merging this pull request may close these issues.

Replace atty dependency in slog-term due to RUSTSEC-2021-0145
5 participants