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

[stable] Fix CVE-2021-42574 and prepare Rust 1.56.1 #90460

Merged
merged 4 commits into from
Nov 1, 2021

Conversation

pietroalbini
Copy link
Member

This PR implements new lints to mitigate the impact of CVE-2021-42574, caused by the presence of bidirectional-override Unicode codepoints in the compiled source code. See the advisory for more information about the vulnerability.

The changes in this PR will be released later today as part of Rust 1.56.1.

@rust-highfive
Copy link
Collaborator

r? @michaelwoerister

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against stable. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2021
@pietroalbini
Copy link
Member Author

The fix was developed by @estebank and reviewed by @nikomatsakis out of band.

@bors r=nikomatsakis p=1000 rollup=never

@bors
Copy link
Contributor

bors commented Nov 1, 2021

📌 Commit 2e7743e has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 1, 2021
@bors
Copy link
Contributor

bors commented Nov 1, 2021

⌛ Testing commit 2e7743e with merge 98024ccb044eacd3c9b07abc09014a70c23dc8b7...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Nov 1, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 1, 2021
@Mark-Simulacrum
Copy link
Member

@bors retry - libSystem.dylib not linked in?

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 1, 2021
@bors
Copy link
Contributor

bors commented Nov 1, 2021

⌛ Testing commit 2e7743e with merge 0003d1a38d97b12a389bd1e5b49f9db92094ee4a...

@rust-log-analyzer

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Nov 1, 2021

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 1, 2021
The test is for an unstable feature that doesn't affect the stable
release (no_core), and it's causing CI issues for macOS.
@pietroalbini
Copy link
Member Author

Disabled the broken test, as it doesn't affect stable (it's for an unstable feature, no_core).

@bors r=nikomatsakis,pietroalbini p=1000 rollup=never

@bors
Copy link
Contributor

bors commented Nov 1, 2021

📌 Commit 6552f7a has been approved by nikomatsakis,pietroalbini

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 1, 2021
@bors
Copy link
Contributor

bors commented Nov 1, 2021

⌛ Testing commit 6552f7a with merge 59eed8a...

@@ -0,0 +1,161 @@
use crate::{EarlyContext, EarlyLintPass, LintContext};
Copy link
Member

Choose a reason for hiding this comment

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

Github says this file contains bidir control characters. This will break once bootstrap gets this lint, right?

Copy link
Member

Choose a reason for hiding this comment

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

Would `#[doc = "...\u{...}..."] work to avoid directly writing the bidir control characters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Uh, that's a really good point, cc @estebank. Good catch!

For the stable and beta patches I'd keep it this way, mostly because this is the patch we shared under embargo with Linux distributions ahead of time and I don't want us to apply a different patch. I'll update the master PR as soon as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok this is a bit harder than I thought since the lint docs generator tool doesn't understand #[doc = ""]. I'll merge the PR to master without that change as I want nightly to be fixed ASAP, we'll take care of this in a future PR.

@bors
Copy link
Contributor

bors commented Nov 1, 2021

☀️ Test successful - checks-actions
Approved by: nikomatsakis,pietroalbini
Pushing 59eed8a to stable...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 1, 2021
@bors bors merged commit 59eed8a into rust-lang:stable Nov 1, 2021
@rustbot rustbot added this to the 1.56.1 milestone Nov 1, 2021
@pietroalbini pietroalbini deleted the bidi-stable branch November 1, 2021 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants