Skip to content

Commit 3bd9bd9

Browse files
committed
Fix handling of .. paths in symlinks.
When `..` appears at the end of a symlink target, but is not at the end of the full path target, don't mark the path as being expected to open a directory. This fixes the reduced testcase in bytecodealliance/wasmtime#9272.
1 parent 076ab49 commit 3bd9bd9

File tree

2 files changed

+118
-3
lines changed

2 files changed

+118
-3
lines changed

cap-primitives/src/fs/manually/open.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,7 @@ impl<'start> Context<'start> {
314314

315315
/// Push the components of `destination` onto the worklist stack.
316316
fn push_symlink_destination(&mut self, destination: PathBuf) -> io::Result<()> {
317+
let at_end = self.components.is_empty();
317318
let trailing_slash = path_has_trailing_slash(&destination);
318319
let trailing_dot = path_has_trailing_dot(&destination);
319320
let trailing_dotdot = destination.ends_with(Component::ParentDir);
@@ -362,9 +363,11 @@ impl<'start> Context<'start> {
362363

363364
// Record whether the new components ended with a path that implies
364365
// an open of `.` at the end of path resolution.
365-
self.follow_with_dot |= trailing_dot | trailing_dotdot;
366-
self.trailing_slash |= trailing_slash;
367-
self.dir_required |= trailing_slash;
366+
if at_end {
367+
self.follow_with_dot |= trailing_dot | trailing_dotdot;
368+
self.trailing_slash |= trailing_slash;
369+
self.dir_required |= trailing_slash;
370+
}
368371

369372
// As an optimization, hold onto the `PathBuf` buffer for later reuse.
370373
self.reuse = destination;

tests/fs_additional.rs

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,3 +1046,115 @@ fn check_metadata(std: &std::fs::Metadata, cap: &cap_std::fs::Metadata) {
10461046
assert_eq!(std.blocks(), cap_std::fs::MetadataExt::blocks(cap));
10471047
}
10481048
}
1049+
1050+
/// Test that a symlink in the middle of a path containing ".." doesn't cause
1051+
/// the path to be treated as if it ends with "..".
1052+
#[test]
1053+
fn dotdot_in_middle_of_symlink() {
1054+
let tmpdir = tmpdir();
1055+
1056+
let foo = b"foo";
1057+
check!(tmpdir.write("target", foo));
1058+
check!(tmpdir.create_dir("b"));
1059+
let b = check!(tmpdir.open_dir("b"));
1060+
check!(b.symlink("..", "up"));
1061+
1062+
let path = "b/up/target";
1063+
let mut file = check!(tmpdir.open(path));
1064+
let mut data = Vec::new();
1065+
check!(file.read_to_end(&mut data));
1066+
assert_eq!(data, foo);
1067+
}
1068+
1069+
/// Same as `dotdot_in_middle_of_symlink`, but use two levels of `..`.
1070+
#[test]
1071+
fn dotdot_more_in_middle_of_symlink() {
1072+
let tmpdir = tmpdir();
1073+
1074+
let foo = b"foo";
1075+
check!(tmpdir.write("target", foo));
1076+
check!(tmpdir.create_dir_all("b/c"));
1077+
let b = check!(tmpdir.open_dir("b"));
1078+
check!(b.symlink("c/../..", "up"));
1079+
1080+
let path = "b/up/target";
1081+
let mut file = check!(tmpdir.open(path));
1082+
let mut data = Vec::new();
1083+
check!(file.read_to_end(&mut data));
1084+
assert_eq!(data, foo);
1085+
}
1086+
1087+
/// Same as `dotdot_more_in_middle_of_symlink`, but use a symlink that
1088+
/// doesn't end with `..`.
1089+
#[test]
1090+
fn dotdot_even_more_in_middle_of_symlink() {
1091+
let tmpdir = tmpdir();
1092+
1093+
let foo = b"foo";
1094+
check!(tmpdir.create_dir_all("b/c"));
1095+
check!(tmpdir.write("b/target", foo));
1096+
let b = check!(tmpdir.open_dir("b"));
1097+
check!(b.symlink("c/../../b", "up"));
1098+
1099+
let path = "b/up/target";
1100+
let mut file = check!(tmpdir.open(path));
1101+
let mut data = Vec::new();
1102+
check!(file.read_to_end(&mut data));
1103+
assert_eq!(data, foo);
1104+
}
1105+
1106+
/// Similar to `dotdot_in_middle_of_symlink`, but this time the symlink to
1107+
/// `..` does happen to be the end of the path, so we need to make sure
1108+
/// the implementation doesn't just do a stack pop when it sees the `..`
1109+
/// leaving us with an `O_PATH` directory handle.
1110+
#[test]
1111+
fn dotdot_at_end_of_symlink() {
1112+
let tmpdir = tmpdir();
1113+
1114+
let foo = b"foo";
1115+
check!(tmpdir.write("target", foo));
1116+
check!(tmpdir.create_dir("b"));
1117+
let b = check!(tmpdir.open_dir("b"));
1118+
check!(b.symlink("..", "up"));
1119+
1120+
// Do some things with `path` that might break with an `O_PATH` fd.
1121+
// On Linux, the `permissions` part doesn't because cap-std uses
1122+
// /proc/self/fd. But the `read_dir` part does.
1123+
let path = "b/up";
1124+
1125+
let perms = check!(tmpdir.metadata(path)).permissions();
1126+
check!(tmpdir.set_permissions(path, perms));
1127+
1128+
let contents = check!(tmpdir.read_dir(path));
1129+
for entry in contents {
1130+
let _entry = check!(entry);
1131+
}
1132+
}
1133+
1134+
/// Like `dotdot_at_end_of_symlink`, but do everything inside a new directory,
1135+
/// so that `MaybeOwnedFile` doesn't reopen `.` which would artificially give
1136+
/// us a non-`O_PATH` fd.
1137+
#[test]
1138+
fn dotdot_at_end_of_symlink_all_inside_dir() {
1139+
let tmpdir = tmpdir();
1140+
1141+
let foo = b"foo";
1142+
check!(tmpdir.create_dir("dir"));
1143+
check!(tmpdir.write("dir/target", foo));
1144+
check!(tmpdir.create_dir("dir/b"));
1145+
let b = check!(tmpdir.open_dir("dir/b"));
1146+
check!(b.symlink("..", "up"));
1147+
1148+
// Do some things with `path` that might break with an `O_PATH` fd.
1149+
// On Linux, the `permissions` part doesn't because cap-std uses
1150+
// /proc/self/fd. But the `read_dir` part does.
1151+
let path = "dir/b/up";
1152+
1153+
let perms = check!(tmpdir.metadata(path)).permissions();
1154+
check!(tmpdir.set_permissions(path, perms));
1155+
1156+
let contents = check!(tmpdir.read_dir(path));
1157+
for entry in contents {
1158+
let _entry = check!(entry);
1159+
}
1160+
}

0 commit comments

Comments
 (0)