Skip to content

Commit 32eb709

Browse files
committed
Implement text_direction_codepoint lint
Adds a lint that detects Unicode BiDi control codepoints (U+202A-U+202E, U+2066-U+2069) in manifest files to prevent Trojan Source attacks (CVE-2021-42574). Default level: deny Fixes #16373 Fixes #16374
1 parent 95f7468 commit 32eb709

File tree

4 files changed

+235
-6
lines changed

4 files changed

+235
-6
lines changed

src/cargo/core/workspace.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use crate::lints::analyze_cargo_lints_table;
2525
use crate::lints::rules::blanket_hint_mostly_unused;
2626
use crate::lints::rules::check_im_a_teapot;
2727
use crate::lints::rules::implicit_minimum_version_req;
28+
use crate::lints::rules::text_direction_codepoint;
2829
use crate::ops;
2930
use crate::sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY, PathSource, SourceConfigMap};
3031
use crate::util::context::{FeatureUnification, Value};
@@ -1313,6 +1314,13 @@ impl<'gctx> Workspace<'gctx> {
13131314
&mut run_error_count,
13141315
self.gctx,
13151316
)?;
1317+
text_direction_codepoint(
1318+
pkg.into(),
1319+
&path,
1320+
&cargo_lints,
1321+
&mut run_error_count,
1322+
self.gctx,
1323+
)?;
13161324

13171325
if run_error_count > 0 {
13181326
let plural = if run_error_count == 1 { "" } else { "s" };
@@ -1370,6 +1378,17 @@ impl<'gctx> Workspace<'gctx> {
13701378
&mut run_error_count,
13711379
self.gctx,
13721380
)?;
1381+
// Only check for BiDi codepoints in virtual workspace manifests.
1382+
// For package roots, the lint is run in emit_pkg_lints.
1383+
if matches!(self.root_maybe(), MaybePackage::Virtual(_)) {
1384+
text_direction_codepoint(
1385+
self.root_maybe().into(),
1386+
self.root_manifest(),
1387+
&cargo_lints,
1388+
&mut run_error_count,
1389+
self.gctx,
1390+
)?;
1391+
}
13731392
}
13741393

13751394
// This is a short term hack to allow `blanket_hint_mostly_unused`

src/cargo/lints/rules/mod.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,19 @@
11
mod blanket_hint_mostly_unused;
22
mod im_a_teapot;
33
mod implicit_minimum_version_req;
4+
mod text_direction_codepoint;
45
mod unknown_lints;
56

67
pub use blanket_hint_mostly_unused::blanket_hint_mostly_unused;
78
pub use im_a_teapot::check_im_a_teapot;
89
pub use implicit_minimum_version_req::implicit_minimum_version_req;
10+
pub use text_direction_codepoint::text_direction_codepoint;
911
pub use unknown_lints::output_unknown_lints;
1012

1113
pub const LINTS: &[crate::lints::Lint] = &[
1214
blanket_hint_mostly_unused::LINT,
1315
implicit_minimum_version_req::LINT,
1416
im_a_teapot::LINT,
17+
text_direction_codepoint::LINT,
1518
unknown_lints::LINT,
1619
];
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
use std::path::Path;
2+
3+
use annotate_snippets::AnnotationKind;
4+
use annotate_snippets::Group;
5+
use annotate_snippets::Level;
6+
use annotate_snippets::Snippet;
7+
use cargo_util_schemas::manifest::TomlToolLints;
8+
9+
use crate::CargoResult;
10+
use crate::GlobalContext;
11+
use crate::lints::Lint;
12+
use crate::lints::LintLevel;
13+
use crate::lints::ManifestFor;
14+
use crate::lints::rel_cwd_manifest_path;
15+
16+
/// Unicode BiDi (bidirectional) control codepoints that can be used in
17+
/// "Trojan Source" attacks (CVE-2021-42574).
18+
///
19+
/// These codepoints change the visual representation of text on screen
20+
/// in a way that does not correspond to their in-memory representation.
21+
const UNICODE_BIDI_CODEPOINTS: &[(char, &str)] = &[
22+
('\u{202A}', "LEFT-TO-RIGHT EMBEDDING"),
23+
('\u{202B}', "RIGHT-TO-LEFT EMBEDDING"),
24+
('\u{202C}', "POP DIRECTIONAL FORMATTING"),
25+
('\u{202D}', "LEFT-TO-RIGHT OVERRIDE"),
26+
('\u{202E}', "RIGHT-TO-LEFT OVERRIDE"),
27+
('\u{2066}', "LEFT-TO-RIGHT ISOLATE"),
28+
('\u{2067}', "RIGHT-TO-LEFT ISOLATE"),
29+
('\u{2068}', "FIRST STRONG ISOLATE"),
30+
('\u{2069}', "POP DIRECTIONAL ISOLATE"),
31+
];
32+
33+
pub const LINT: Lint = Lint {
34+
name: "text_direction_codepoint",
35+
desc: "unicode codepoint changing visible direction of text present in manifest",
36+
groups: &[],
37+
default_level: LintLevel::Deny,
38+
edition_lint_opts: None,
39+
feature_gate: None,
40+
docs: None,
41+
};
42+
43+
pub fn text_direction_codepoint(
44+
manifest: ManifestFor<'_>,
45+
manifest_path: &Path,
46+
cargo_lints: &TomlToolLints,
47+
error_count: &mut usize,
48+
gctx: &GlobalContext,
49+
) -> CargoResult<()> {
50+
let (lint_level, reason) = manifest.lint_level(cargo_lints, LINT);
51+
52+
if lint_level == LintLevel::Allow {
53+
return Ok(());
54+
}
55+
56+
let contents = manifest.contents();
57+
let manifest_path_str = rel_cwd_manifest_path(manifest_path, gctx);
58+
59+
let findings: Vec<_> = contents
60+
.char_indices()
61+
.filter_map(|(idx, ch)| {
62+
UNICODE_BIDI_CODEPOINTS
63+
.iter()
64+
.find(|(c, _)| *c == ch)
65+
.map(|(_, name)| (idx, ch, *name))
66+
})
67+
.collect();
68+
69+
if findings.is_empty() {
70+
return Ok(());
71+
}
72+
73+
let level = lint_level.to_diagnostic_level();
74+
let emitted_source = LINT.emitted_source(lint_level, reason);
75+
76+
for (i, (byte_idx, ch, name)) in findings.iter().enumerate() {
77+
if lint_level.is_error() {
78+
*error_count += 1;
79+
}
80+
81+
let title = format!("{}: `\\u{{{:04X}}}` ({})", LINT.desc, *ch as u32, name);
82+
let span = *byte_idx..(*byte_idx + ch.len_utf8());
83+
84+
let mut group = Group::with_title(level.clone().primary_title(&title)).element(
85+
Snippet::source(contents)
86+
.path(&manifest_path_str)
87+
.annotation(
88+
AnnotationKind::Primary
89+
.span(span)
90+
.label("this invisible unicode codepoint changes text flow direction"),
91+
),
92+
);
93+
94+
if i == 0 {
95+
group = group.element(Level::NOTE.message(&emitted_source));
96+
}
97+
group = group.element(Level::HELP.message(
98+
"if their presence wasn't intentional, you can remove them",
99+
));
100+
101+
gctx.shell().print_report(&[group], lint_level.force())?;
102+
}
103+
104+
Ok(())
105+
}
106+

tests/testsuite/lints/text_direction_codepoint.rs

Lines changed: 107 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,17 @@ description = \"A \u{202E}test package\"
1919

2020
p.cargo("check -Zcargo-lints")
2121
.masquerade_as_nightly_cargo(&["cargo-lints"])
22+
.with_status(101)
2223
.with_stderr_data(str![[r#"
23-
[CHECKING] foo v0.0.1 ([ROOT]/foo)
24-
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
24+
[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
25+
--> Cargo.toml:6:18
26+
|
27+
6 | description = "A �test package"
28+
| ^ this invisible unicode codepoint changes text flow direction
29+
|
30+
= [NOTE] `cargo::text_direction_codepoint` is set to `deny` by default
31+
= [HELP] if their presence wasn't intentional, you can remove them
32+
[ERROR] encountered 1 error while running lints
2533
2634
"#]])
2735
.run();
@@ -36,6 +44,77 @@ name = \"foo\"
3644
version = \"0.0.1\"
3745
edition = \"2015\"
3846
description = \"A \u{202E}test\u{202D} package\"
47+
";
48+
let p = project()
49+
.file("Cargo.toml", manifest)
50+
.file("src/lib.rs", "")
51+
.build();
52+
53+
p.cargo("check -Zcargo-lints")
54+
.masquerade_as_nightly_cargo(&["cargo-lints"])
55+
.with_status(101)
56+
.with_stderr_data(str![[r#"
57+
[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
58+
--> Cargo.toml:6:18
59+
|
60+
6 | description = "A �test� package"
61+
| ^ this invisible unicode codepoint changes text flow direction
62+
|
63+
= [NOTE] `cargo::text_direction_codepoint` is set to `deny` by default
64+
= [HELP] if their presence wasn't intentional, you can remove them
65+
[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202D}` (LEFT-TO-RIGHT OVERRIDE)
66+
--> Cargo.toml:6:23
67+
|
68+
6 | description = "A �test� package"
69+
| ^ this invisible unicode codepoint changes text flow direction
70+
|
71+
= [HELP] if their presence wasn't intentional, you can remove them
72+
[ERROR] encountered 2 errors while running lints
73+
74+
"#]])
75+
.run();
76+
}
77+
78+
#[cargo_test]
79+
fn allow_lint() {
80+
// Test that the lint can be allowed
81+
let manifest = "
82+
[package]
83+
name = \"foo\"
84+
version = \"0.0.1\"
85+
edition = \"2015\"
86+
description = \"A \u{202E}test package\"
87+
88+
[lints.cargo]
89+
text_direction_codepoint = \"allow\"
90+
";
91+
let p = project()
92+
.file("Cargo.toml", manifest)
93+
.file("src/lib.rs", "")
94+
.build();
95+
96+
p.cargo("check -Zcargo-lints")
97+
.masquerade_as_nightly_cargo(&["cargo-lints"])
98+
.with_stderr_data(str![[r#"
99+
[CHECKING] foo v0.0.1 ([ROOT]/foo)
100+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
101+
102+
"#]])
103+
.run();
104+
}
105+
106+
#[cargo_test]
107+
fn warn_lint() {
108+
// Test that the lint can be set to warn
109+
let manifest = "
110+
[package]
111+
name = \"foo\"
112+
version = \"0.0.1\"
113+
edition = \"2015\"
114+
description = \"A \u{202E}test package\"
115+
116+
[lints.cargo]
117+
text_direction_codepoint = \"warn\"
39118
";
40119
let p = project()
41120
.file("Cargo.toml", manifest)
@@ -45,6 +124,14 @@ description = \"A \u{202E}test\u{202D} package\"
45124
p.cargo("check -Zcargo-lints")
46125
.masquerade_as_nightly_cargo(&["cargo-lints"])
47126
.with_stderr_data(str![[r#"
127+
[WARNING] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
128+
--> Cargo.toml:6:18
129+
|
130+
6 | description = "A �test package"
131+
| ^ this invisible unicode codepoint changes text flow direction
132+
|
133+
= [NOTE] `cargo::text_direction_codepoint` is set to `warn` in `[lints]`
134+
= [HELP] if their presence wasn't intentional, you can remove them
48135
[CHECKING] foo v0.0.1 ([ROOT]/foo)
49136
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
50137
@@ -79,18 +166,24 @@ edition = "2015"
79166
}
80167

81168
#[cargo_test]
82-
fn workspace_member_bidi() {
83-
// Workspace with BiDi in member package
169+
fn workspace_inherited_allow() {
170+
// Workspace-level lint configuration with member package
84171
let manifest = "
85172
[workspace]
86173
members = [\"foo\"]
174+
175+
[workspace.lints.cargo]
176+
text_direction_codepoint = \"allow\"
87177
";
88178
let foo_manifest = "
89179
[package]
90180
name = \"foo\"
91181
version = \"0.0.1\"
92182
edition = \"2015\"
93183
description = \"A \u{202E}test package\"
184+
185+
[lints]
186+
workspace = true
94187
";
95188
let p = project()
96189
.file("Cargo.toml", manifest)
@@ -132,9 +225,17 @@ edition = "2015"
132225

133226
p.cargo("check -Zcargo-lints")
134227
.masquerade_as_nightly_cargo(&["cargo-lints"])
228+
.with_status(101)
135229
.with_stderr_data(str![[r#"
136-
[CHECKING] foo v0.0.1 ([ROOT]/foo/foo)
137-
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
230+
[ERROR] unicode codepoint changing visible direction of text present in manifest: `/u{202E}` (RIGHT-TO-LEFT OVERRIDE)
231+
--> Cargo.toml:6:14
232+
|
233+
6 | info = "test �info"
234+
| ^ this invisible unicode codepoint changes text flow direction
235+
|
236+
= [NOTE] `cargo::text_direction_codepoint` is set to `deny` by default
237+
= [HELP] if their presence wasn't intentional, you can remove them
238+
[ERROR] encountered 1 error while running lints
138239
139240
"#]])
140241
.run();

0 commit comments

Comments
 (0)