Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ thiserror = "2.0.18"
time = { version = "0.3.47", features = ["parsing", "formatting", "serde"] }
toml = { version = "1.1.2", default-features = false }
toml_edit = { version = "0.25.10", features = ["serde"] }
toml_parser = "1.1.2"
toml_writer = "1.1.1"
tracing = { version = "0.1.44", default-features = false, features = ["std"] } # be compatible with rustc_log: https://github.com/rust-lang/rust/blob/e51e98dde6a/compiler/rustc_log/Cargo.toml#L9
tracing-chrome = "0.7.2"
tracing-subscriber = { version = "0.3.23", features = ["env-filter"] }
Expand Down Expand Up @@ -227,6 +229,8 @@ thiserror.workspace = true
time.workspace = true
toml = { workspace = true, features = ["std", "serde", "parse", "display", "preserve_order"] }
toml_edit.workspace = true
toml_parser.workspace = true
toml_writer.workspace = true
tracing = { workspace = true, features = ["attributes"] }
tracing-chrome.workspace = true
tracing-subscriber.workspace = true
Expand Down
30 changes: 30 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ use crate::lints::rules::non_snake_case_features;
use crate::lints::rules::non_snake_case_packages;
use crate::lints::rules::redundant_homepage;
use crate::lints::rules::redundant_readme;
use crate::lints::rules::text_direction_codepoint_in_comment;
use crate::lints::rules::text_direction_codepoint_in_literal;
use crate::lints::rules::unused_build_dependencies_no_build_rs;
use crate::lints::rules::unused_workspace_dependencies;
use crate::lints::rules::unused_workspace_package_fields;
Expand Down Expand Up @@ -1401,6 +1403,20 @@ impl<'gctx> Workspace<'gctx> {
&mut run_error_count,
self.gctx,
)?;
text_direction_codepoint_in_comment(
pkg.into(),
&path,
&cargo_lints,
&mut run_error_count,
self.gctx,
)?;
text_direction_codepoint_in_literal(
pkg.into(),
&path,
&cargo_lints,
&mut run_error_count,
self.gctx,
)?;

if run_error_count > 0 {
let plural = if run_error_count == 1 { "" } else { "s" };
Expand Down Expand Up @@ -1475,6 +1491,20 @@ impl<'gctx> Workspace<'gctx> {
&mut run_error_count,
self.gctx,
)?;
text_direction_codepoint_in_comment(
(self, self.root_maybe()).into(),
self.root_manifest(),
&cargo_lints,
&mut run_error_count,
self.gctx,
)?;
text_direction_codepoint_in_literal(
(self, self.root_maybe()).into(),
self.root_manifest(),
&cargo_lints,
&mut run_error_count,
self.gctx,
)?;
}

// This is a short term hack to allow `blanket_hint_mostly_unused`
Expand Down
6 changes: 6 additions & 0 deletions src/cargo/lints/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ mod non_snake_case_features;
mod non_snake_case_packages;
mod redundant_homepage;
mod redundant_readme;
mod text_direction_codepoint_in_comment;
mod text_direction_codepoint_in_literal;
mod unknown_lints;
pub mod unused_dependencies;
mod unused_workspace_dependencies;
Expand All @@ -26,6 +28,8 @@ pub use non_snake_case_features::non_snake_case_features;
pub use non_snake_case_packages::non_snake_case_packages;
pub use redundant_homepage::redundant_homepage;
pub use redundant_readme::redundant_readme;
pub use text_direction_codepoint_in_comment::text_direction_codepoint_in_comment;
pub use text_direction_codepoint_in_literal::text_direction_codepoint_in_literal;
pub use unknown_lints::output_unknown_lints;
pub use unused_dependencies::unused_build_dependencies_no_build_rs;
pub use unused_workspace_dependencies::unused_workspace_dependencies;
Expand All @@ -43,6 +47,8 @@ pub static LINTS: &[&crate::lints::Lint] = &[
non_snake_case_packages::LINT,
redundant_homepage::LINT,
redundant_readme::LINT,
text_direction_codepoint_in_comment::LINT,
text_direction_codepoint_in_literal::LINT,
unknown_lints::LINT,
unused_dependencies::LINT,
unused_workspace_dependencies::LINT,
Expand Down
192 changes: 192 additions & 0 deletions src/cargo/lints/rules/text_direction_codepoint_in_comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
use std::path::Path;

use cargo_util_schemas::manifest::TomlToolLints;
use cargo_util_terminal::report::AnnotationKind;
use cargo_util_terminal::report::Group;
use cargo_util_terminal::report::Level;
use cargo_util_terminal::report::Snippet;
use toml_parser::Source;
use toml_parser::Span;
use toml_parser::decoder::Encoding;
use toml_parser::parser::Event;
use toml_parser::parser::EventKind;
use toml_parser::parser::EventReceiver;

use crate::CargoResult;
use crate::GlobalContext;
use crate::core::MaybePackage;
use crate::lints::CORRECTNESS;
use crate::lints::Lint;
use crate::lints::LintLevel;
use crate::lints::ManifestFor;
use crate::lints::rel_cwd_manifest_path;

pub static LINT: &Lint = &Lint {
name: "text_direction_codepoint_in_comment",
desc: "unicode codepoint changing visible direction of text present in comment",
primary_group: &CORRECTNESS,
msrv: Some(super::CARGO_LINTS_MSRV),
feature_gate: None,
docs: Some(
r#"
### What it does
Detects Unicode codepoints in manifest comments that change the visual representation of text on screen
in a way that does not correspond to their on memory representation.

### Why it is bad
Unicode allows changing the visual flow of text on screen
in order to support scripts that are written right-to-left,
but a specially crafted comment can make code that will be compiled appear to be part of a comment,
depending on the software used to read the code.
To avoid potential problems or confusion,
such as in CVE-2021-42574,
by default we deny their use.
"#,
),
};

pub fn text_direction_codepoint_in_comment(
manifest: ManifestFor<'_>,
manifest_path: &Path,
cargo_lints: &TomlToolLints,
error_count: &mut usize,
gctx: &GlobalContext,
) -> CargoResult<()> {
let (lint_level, source) = manifest.lint_level(cargo_lints, LINT);
if lint_level == LintLevel::Allow {
return Ok(());
}

if matches!(
&manifest,
ManifestFor::Workspace {
maybe_pkg: MaybePackage::Package { .. },
..
}
) {
// For real manifests, lint as a package, rather than a workspace
return Ok(());
}

let Some(contents) = manifest.contents() else {
return Ok(());
};

let bidi_spans = contents
.char_indices()
.filter(|(_i, c)| {
UNICODE_BIDI_CODEPOINTS
.iter()
.any(|(bidi, _name)| c == bidi)
})
.map(|(i, c)| (i, i + c.len_utf8()))
.collect::<Vec<_>>();
if bidi_spans.is_empty() {
return Ok(());
}

let events = bidi_events(contents, &bidi_spans);
let manifest_path = rel_cwd_manifest_path(manifest_path, gctx);
let mut emitted_source = None;
for event in events {
if lint_level.is_error() {
*error_count += 1;
}

let token_span = event.token.span();
let token_span = token_span.start()..token_span.end();
let mut snippet = Snippet::source(contents).path(&manifest_path).annotation(
AnnotationKind::Context
.span(token_span)
.label("this comment contains an invisible unicode text flow control codepoint"),
);
for bidi_span in event.bidi_spans {
let bidi_span = bidi_span.0..bidi_span.1;
let escaped = format!("{:?}", &contents[bidi_span.clone()]);
snippet = snippet.annotation(AnnotationKind::Primary.span(bidi_span).label(escaped));
}

let level = lint_level.to_diagnostic_level();
let mut primary = Group::with_title(level.primary_title(LINT.desc)).element(snippet);
if emitted_source.is_none() {
emitted_source = Some(LINT.emitted_source(lint_level, source));
primary = primary.element(Level::NOTE.message(emitted_source.as_ref().unwrap()));
}

let report = [primary];
gctx.shell().print_report(&report, lint_level.force())?;
}

Ok(())
}

const UNICODE_BIDI_CODEPOINTS: &[(char, &str)] = &[
('\u{202A}', "LEFT-TO-RIGHT EMBEDDING"),
('\u{202B}', "RIGHT-TO-LEFT EMBEDDING"),
('\u{202C}', "POP DIRECTIONAL FORMATTING"),
('\u{202D}', "LEFT-TO-RIGHT OVERRIDE"),
('\u{202E}', "RIGHT-TO-LEFT OVERRIDE"),
('\u{2066}', "LEFT-TO-RIGHT ISOLATE"),
('\u{2067}', "RIGHT-TO-LEFT ISOLATE"),
('\u{2068}', "FIRST STRONG ISOLATE"),
('\u{2069}', "POP DIRECTIONAL ISOLATE"),
];

struct BiDiEvent {
token: Event,
bidi_spans: Vec<(usize, usize)>,
}

fn bidi_events(contents: &str, bidi_spans: &[(usize, usize)]) -> Vec<BiDiEvent> {
let mut bidi_spans = bidi_spans.iter();
let bidi_span = bidi_spans.next().copied();

let source = Source::new(contents);
let tokens = source.lex().into_vec();
let mut collector = BiDiCollector {
bidi_span,
bidi_spans,
events: Vec::new(),
};
let mut errors = ();
toml_parser::parser::parse_document(&tokens, &mut collector, &mut errors);

collector.events
}

struct BiDiCollector<'b> {
bidi_span: Option<(usize, usize)>,
bidi_spans: std::slice::Iter<'b, (usize, usize)>,
events: Vec<BiDiEvent>,
}

impl BiDiCollector<'_> {
fn process(&mut self, kind: EventKind, encoding: Option<Encoding>, span: Span) {
let mut event_bidi_spans = Vec::new();
while let Some(bidi_span) = self.bidi_span {
if bidi_span.0 < span.start() {
self.bidi_span = self.bidi_spans.next().copied();
continue;
} else if span.end() <= bidi_span.0 {
break;
}

event_bidi_spans.push(bidi_span);
self.bidi_span = self.bidi_spans.next().copied();
}

if !event_bidi_spans.is_empty() {
let token = Event::new_unchecked(kind, encoding, span);
self.events.push(BiDiEvent {
token,
bidi_spans: event_bidi_spans,
});
}
}
}

impl EventReceiver for BiDiCollector<'_> {
fn comment(&mut self, span: Span, _error: &mut dyn toml_parser::ErrorSink) {
self.process(EventKind::Comment, None, span)
}
}
Loading