Implement text_direction_codepoint lint

Detects Unicode BiDi control codepoints in Cargo.toml files to
mitigate Trojan Source attacks (CVE-2021-42574).

Features:
- Detects 9 Unicode BiDi control codepoints
- Groups multiple codepoints on same line into single error (like rustc)
- Moves workspace check into lint function for cleaner architecture
- Uses rustc-style diagnostics with codepoint info in label annotation
- Includes explanatory note about BiDi codepoint risks
- Deny-by-default lint level

Author: enthropy7

.DS_Store

@@ -109,6 +109,7 @@ thiserror = "2.0.17"
 time = { version = "0.3.44", features = ["parsing", "formatting", "serde"] }
 toml = { version = "0.9.10", default-features = false }
 toml_edit = { version = "0.24.0", features = ["serde"] }
+toml_parser = "1.0.6"
 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.22", features = ["env-filter"] }
@@ -214,6 +215,7 @@ thiserror.workspace = true
 time.workspace = true
 toml = { workspace = true, features = ["std", "serde", "parse", "display", "preserve_order"] }
 toml_edit.workspace = true
+toml_parser.workspace = true
 tracing = { workspace = true, features = ["attributes"] }
 tracing-subscriber.workspace = true
 unicase.workspace = true

Cargo.lock

@@ -25,6 +25,7 @@ use crate::lints::analyze_cargo_lints_table;
 use crate::lints::rules::blanket_hint_mostly_unused;
 use crate::lints::rules::check_im_a_teapot;
 use crate::lints::rules::implicit_minimum_version_req;
+use crate::lints::rules::text_direction_codepoint;
 use crate::ops;
 use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY, PathSource, SourceConfigMap};
 use crate::util::context::{FeatureUnification, Value};
@@ -1313,6 +1314,13 @@ impl<'gctx> Workspace<'gctx> {
                 &mut run_error_count,
                 self.gctx,
             )?;
+            text_direction_codepoint(
+                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" };
@@ -1370,6 +1378,13 @@ impl<'gctx> Workspace<'gctx> {
                 &mut run_error_count,
                 self.gctx,
             )?;
+            text_direction_codepoint(
+                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`

Cargo.toml

@@ -1,16 +1,19 @@
 mod blanket_hint_mostly_unused;
 mod im_a_teapot;
 mod implicit_minimum_version_req;
+mod text_direction_codepoint;
 mod unknown_lints;
 
 pub use blanket_hint_mostly_unused::blanket_hint_mostly_unused;
 pub use im_a_teapot::check_im_a_teapot;
 pub use implicit_minimum_version_req::implicit_minimum_version_req;
+pub use text_direction_codepoint::text_direction_codepoint;
 pub use unknown_lints::output_unknown_lints;
 
 pub const LINTS: &[crate::lints::Lint] = &[
     blanket_hint_mostly_unused::LINT,
     implicit_minimum_version_req::LINT,
     im_a_teapot::LINT,
+    text_direction_codepoint::LINT,
     unknown_lints::LINT,
 ];

benches/.DS_Store

@@ -0,0 +1,282 @@
+use std::path::Path;
+
+use annotate_snippets::AnnotationKind;
+use annotate_snippets::Group;
+use annotate_snippets::Level;
+use annotate_snippets::Snippet;
+use cargo_util_schemas::manifest::TomlToolLints;
+use toml_parser::Source;
+use toml_parser::Span;
+use toml_parser::parser::EventReceiver;
+
+use crate::CargoResult;
+use crate::GlobalContext;
+use crate::core::MaybePackage;
+use crate::lints::Lint;
+use crate::lints::LintLevel;
+use crate::lints::ManifestFor;
+use crate::lints::rel_cwd_manifest_path;
+
+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"),
+];
+
+pub const LINT: Lint = Lint {
+    name: "text_direction_codepoint",
+    desc: "unicode codepoint changing visible direction of text present in manifest",
+    groups: &[],
+    default_level: LintLevel::Deny,
+    edition_lint_opts: None,
+    feature_gate: None,
+    docs: None,
+};
+
+/// Paths where BiDi codepoints are allowed (legitimate RTL content).
+const ALLOWED_BIDI_PATHS: &[&[&str]] = &[
+    &["package", "description"],
+    &["package", "metadata"],
+    &["workspace", "metadata"],
+];
+
+fn is_allowed_bidi_path(path: &[String]) -> bool {
+    ALLOWED_BIDI_PATHS.iter().any(|allowed| {
+        if path.len() < allowed.len() {
+            return false;
+        }
+        path.iter()
+            .zip(allowed.iter())
+            .all(|(a, b)| a.as_str() == *b)
+    })
+}
+
+struct Finding {
+    byte_idx: usize,
+    ch: char,
+    name: &'static str,
+    in_allowed_value: bool,
+}
+
+struct BiDiCollector<'a> {
+    contents: &'a str,
+    findings: &'a mut Vec<Finding>,
+    key_path: Vec<String>,
+    table_stack: Vec<Vec<String>>,
+}
+
+impl<'a> BiDiCollector<'a> {
+    fn new(contents: &'a str, findings: &'a mut Vec<Finding>) -> Self {
+        Self {
+            contents,
+            findings,
+            key_path: Vec::new(),
+            table_stack: Vec::new(),
+        }
+    }
+
+    fn check_span_for_bidi(&mut self, span: Span, in_value: bool) {
+        let text = &self.contents[span.start()..span.end()];
+        for (offset, ch) in text.char_indices() {
+            if let Some((_, name)) = UNICODE_BIDI_CODEPOINTS.iter().find(|(c, _)| *c == ch) {
+                let in_allowed_value = in_value && is_allowed_bidi_path(&self.key_path);
+                self.findings.push(Finding {
+                    byte_idx: span.start() + offset,
+                    ch,
+                    name,
+                    in_allowed_value,
+                });
+            }
+        }
+    }
+
+    fn current_path(&self) -> Vec<String> {
+        let mut path = self.table_stack.last().cloned().unwrap_or_default();
+        path.extend(self.key_path.clone());
+        path
+    }
+}
+
+impl EventReceiver for BiDiCollector<'_> {
+    fn std_table_open(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {
+        self.table_stack.push(self.key_path.clone());
+        self.key_path.clear();
+    }
+
+    fn std_table_close(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {
+        if let Some(last) = self.table_stack.last_mut() {
+            *last = self.key_path.clone();
+        }
+        self.key_path.clear();
+    }
+
+    fn array_table_open(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {
+        self.table_stack.push(self.key_path.clone());
+        self.key_path.clear();
+    }
+
+    fn array_table_close(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {
+        if let Some(last) = self.table_stack.last_mut() {
+            *last = self.key_path.clone();
+        }
+        self.key_path.clear();
+    }
+
+    fn simple_key(
+        &mut self,
+        span: Span,
+        _kind: Option<toml_parser::decoder::Encoding>,
+        _error: &mut dyn toml_parser::ErrorSink,
+    ) {
+        self.check_span_for_bidi(span, false);
+
+        let key_text = &self.contents[span.start()..span.end()];
+        let key = if (key_text.starts_with('"') && key_text.ends_with('"'))
+            || (key_text.starts_with('\'') && key_text.ends_with('\''))
+        {
+            key_text[1..key_text.len() - 1].to_string()
+        } else {
+            key_text.to_string()
+        };
+        self.key_path.push(key);
+    }
+
+    fn key_sep(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {}
+
+    fn key_val_sep(&mut self, _span: Span, _error: &mut dyn toml_parser::ErrorSink) {}
+
+    fn scalar(
+        &mut self,
+        span: Span,
+        _kind: Option<toml_parser::decoder::Encoding>,
+        _error: &mut dyn toml_parser::ErrorSink,
+    ) {
+        let full_path = self.current_path();
+        let saved_key_path = std::mem::replace(&mut self.key_path, full_path);
+        self.check_span_for_bidi(span, true);
+        self.key_path = saved_key_path;
+        self.key_path.clear();
+    }
+
+    fn comment(&mut self, span: Span, _error: &mut dyn toml_parser::ErrorSink) {
+        self.check_span_for_bidi(span, false);
+    }
+}
+
+pub fn text_direction_codepoint(
+    manifest: ManifestFor<'_>,
+    manifest_path: &Path,
+    cargo_lints: &TomlToolLints,
+    error_count: &mut usize,
+    gctx: &GlobalContext,
+) -> CargoResult<()> {
+    let (lint_level, reason) = manifest.lint_level(cargo_lints, LINT);
+
+    if lint_level == LintLevel::Allow {
+        return Ok(());
+    }
+
+    if matches!(&manifest, ManifestFor::Workspace(MaybePackage::Package(_))) {
+        return Ok(());
+    }
+
+    let contents = manifest.contents();
+
+    let has_bidi = contents.chars().any(|ch| {
+        UNICODE_BIDI_CODEPOINTS
+            .iter()
+            .any(|(bidi_ch, _)| *bidi_ch == ch)
+    });
+
+    if !has_bidi {
+        return Ok(());
+    }
+
+    let mut findings = Vec::new();
+    {
+        let source = Source::new(contents);
+        let tokens = source.lex().into_vec();
+        let mut collector = BiDiCollector::new(contents, &mut findings);
+        let mut errors = Vec::new();
+        toml_parser::parser::parse_document(&tokens, &mut collector, &mut errors);
+    }
+
+    let disallowed_findings: Vec<_> = findings
+        .into_iter()
+        .filter(|f| !f.in_allowed_value)
+        .collect();
+
+    if disallowed_findings.is_empty() {
+        return Ok(());
+    }
+
+    let manifest_path_str = rel_cwd_manifest_path(manifest_path, gctx);
+
+    let mut line_map = Vec::new();
+    line_map.push(0);
+    for (idx, ch) in contents.char_indices() {
+        if ch == '\n' {
+            line_map.push(idx + 1);
+        }
+    }
+
+    let mut findings_by_line: std::collections::BTreeMap<usize, Vec<_>> =
+        std::collections::BTreeMap::new();
+    for finding in disallowed_findings {
+        let line_num = line_map
+            .iter()
+            .rposition(|&line_start| line_start <= finding.byte_idx)
+            .map(|i| i + 1)
+            .unwrap_or(1);
+        findings_by_line
+            .entry(line_num)
+            .or_insert_with(Vec::new)
+            .push(finding);
+    }
+
+    let level = lint_level.to_diagnostic_level();
+    let emitted_source = LINT.emitted_source(lint_level, reason);
+
+    for (line_idx, line_findings) in findings_by_line.values().enumerate() {
+        if lint_level.is_error() {
+            *error_count += 1;
+        }
+
+        let title = LINT.desc.to_string();
+
+        let labels: Vec<String> = line_findings
+            .iter()
+            .map(|f| format!(r#"`\u{{{:04X}}}` ({})"#, f.ch as u32, f.name))
+            .collect();
+
+        let mut snippet = Snippet::source(contents).path(&manifest_path_str);
+        for (finding, label) in line_findings.iter().zip(labels.iter()) {
+            let span = finding.byte_idx..(finding.byte_idx + finding.ch.len_utf8());
+            snippet = snippet.annotation(AnnotationKind::Primary.span(span).label(label));
+        }
+
+        let mut group = Group::with_title(level.clone().primary_title(&title)).element(snippet);
+
+        if line_idx == 0 {
+            group = group.element(Level::NOTE.message(&emitted_source));
+            group = group.element(Level::NOTE.message(
+                "these kinds of unicode codepoints change the way text flows on screen, \
+                 but can cause confusion because they change the order of characters",
+            ));
+        }
+        group = group.element(Level::HELP.message(
+            "if their presence wasn't intentional, you can remove them, \
+             or use their escape sequence (e.g., \\u{202E}) in double-quoted strings",
+        ));
+
+        gctx.shell().print_report(&[group], lint_level.force())?;
+    }
+
+    Ok(())
+}