Skip to content

Commit 0d0acdf

Browse files
lints: add check for non-utf-8 filesystem content
As mentioned in #975, we should add a lint to ensure that all filenames are UTF-8, giving nice errors (with full pathnames) in the event that we encounter invalid filenames. We also check symlink targets. Add a unit test that tries various valid and invalid scenarios. Signed-off-by: Allison Karlitskaya <allison.karlitskaya@redhat.com>
1 parent 6759010 commit 0d0acdf

File tree

1 file changed

+107
-2
lines changed

1 file changed

+107
-2
lines changed

lib/src/lints.rs

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
55
use std::env::consts::ARCH;
66

7-
use anyhow::Result;
7+
use anyhow::{bail, ensure, Result};
88
use cap_std::fs::Dir;
99
use cap_std_ext::cap_std;
1010
use cap_std_ext::dirext::CapStdExtDirExt as _;
@@ -15,7 +15,13 @@ use fn_error_context::context;
1515
/// if it does not exist error.
1616
#[context("Linting")]
1717
pub(crate) fn lint(root: &Dir) -> Result<()> {
18-
let lints = [check_var_run, check_kernel, check_parse_kargs, check_usretc];
18+
let lints = [
19+
check_var_run,
20+
check_kernel,
21+
check_parse_kargs,
22+
check_usretc,
23+
check_utf8,
24+
];
1925
for lint in lints {
2026
lint(&root)?;
2127
}
@@ -59,6 +65,33 @@ fn check_kernel(root: &Dir) -> Result<()> {
5965
Ok(())
6066
}
6167

68+
fn check_utf8(dir: &Dir) -> Result<()> {
69+
for entry in dir.entries()? {
70+
let entry = entry?;
71+
let name = entry.file_name();
72+
73+
let Some(strname) = &name.to_str() else {
74+
// will escape nicely like "abc\xFFdéf"
75+
bail!("/: Found non-utf8 filename {name:?}");
76+
};
77+
78+
let ifmt = entry.file_type()?;
79+
if ifmt.is_symlink() {
80+
let target = dir.read_link_contents(&name)?;
81+
ensure!(
82+
target.to_str().is_some(),
83+
"/{strname}: Found non-utf8 symlink target"
84+
);
85+
} else if ifmt.is_dir() {
86+
if let Err(err) = check_utf8(&entry.open_dir()?) {
87+
// Try to do the path pasting only in the event of an error
88+
bail!("/{strname}{err:?}");
89+
}
90+
}
91+
}
92+
Ok(())
93+
}
94+
6295
#[cfg(test)]
6396
fn fixture() -> Result<cap_std_ext::cap_tempfile::TempDir> {
6497
let tempdir = cap_std_ext::cap_tempfile::tempdir(cap_std::ambient_authority())?;
@@ -117,3 +150,75 @@ fn test_usr_etc() -> Result<()> {
117150
check_usretc(root).unwrap();
118151
Ok(())
119152
}
153+
154+
#[test]
155+
fn test_non_utf8() {
156+
use std::{ffi::OsStr, os::unix::ffi::OsStrExt};
157+
158+
let root = &fixture().unwrap();
159+
160+
// Try to create some adversarial symlink situations to ensure the walk doesn't crash
161+
root.create_dir("subdir").unwrap();
162+
// Self-referential symlinks
163+
root.symlink("self", "self").unwrap();
164+
// Infinitely looping dir symlinks
165+
root.symlink("..", "subdir/parent").unwrap();
166+
// Broken symlinks
167+
root.symlink("does-not-exist", "broken").unwrap();
168+
// Out-of-scope symlinks
169+
root.symlink("../../x", "escape").unwrap();
170+
// Should be fine
171+
check_utf8(root).unwrap();
172+
173+
// But this will cause an issue
174+
let baddir = OsStr::from_bytes(b"subdir/2/bad\xffdir");
175+
root.create_dir("subdir/2").unwrap();
176+
root.create_dir(baddir).unwrap();
177+
let Err(err) = check_utf8(root) else {
178+
unreachable!("Didn't fail");
179+
};
180+
assert_eq!(
181+
err.to_string(),
182+
r#"/subdir/2/: Found non-utf8 filename "bad\xFFdir""#
183+
);
184+
root.remove_dir(baddir).unwrap(); // Get rid of the problem
185+
check_utf8(root).unwrap(); // Check it
186+
187+
// Create a new problem in the form of a regular file
188+
let badfile = OsStr::from_bytes(b"regular\xff");
189+
root.write(badfile, b"Hello, world!\n").unwrap();
190+
let Err(err) = check_utf8(root) else {
191+
unreachable!("Didn't fail");
192+
};
193+
assert_eq!(
194+
err.to_string(),
195+
r#"/: Found non-utf8 filename "regular\xFF""#
196+
);
197+
root.remove_file(badfile).unwrap(); // Get rid of the problem
198+
check_utf8(root).unwrap(); // Check it
199+
200+
// And now test invalid symlink targets
201+
root.symlink(badfile, "subdir/good-name").unwrap();
202+
let Err(err) = check_utf8(root) else {
203+
unreachable!("Didn't fail");
204+
};
205+
assert_eq!(
206+
err.to_string(),
207+
r#"/subdir/good-name: Found non-utf8 symlink target"#
208+
);
209+
root.remove_file("subdir/good-name").unwrap(); // Get rid of the problem
210+
check_utf8(root).unwrap(); // Check it
211+
212+
// Finally, test a self-referential symlink with an invalid name.
213+
// We should spot the invalid name before we check the target.
214+
root.symlink(badfile, badfile).unwrap();
215+
let Err(err) = check_utf8(root) else {
216+
unreachable!("Didn't fail");
217+
};
218+
assert_eq!(
219+
err.to_string(),
220+
r#"/: Found non-utf8 filename "regular\xFF""#
221+
);
222+
root.remove_file(badfile).unwrap(); // Get rid of the problem
223+
check_utf8(root).unwrap(); // Check it
224+
}

0 commit comments

Comments
 (0)