Disallow creating files with '..' (#3156)

Release Notes:

- Fixed a crash that could occur when creating files with '..' in the
path

crates/collab/src/tests/integration_tests.rs

@@ -4936,10 +4936,10 @@ async fn test_project_symbols(
         .await
         .unwrap();
 
-    buffer_b_2.read_with(cx_b, |buffer, _| {
+    buffer_b_2.read_with(cx_b, |buffer, cx| {
         assert_eq!(
-            buffer.file().unwrap().path().as_ref(),
-            Path::new("../crate-2/two.rs")
+            buffer.file().unwrap().full_path(cx),
+            Path::new("/code/crate-2/two.rs")
         );
     });
 

crates/file_finder/src/file_finder.rs

@@ -1297,7 +1297,7 @@ mod tests {
         // so that one should be sorted earlier
         let b_path = ProjectPath {
             worktree_id,
-            path: Arc::from(Path::new("/root/dir2/b.txt")),
+            path: Arc::from(Path::new("dir2/b.txt")),
         };
         workspace
             .update(cx, |workspace, cx| {

crates/project/src/project.rs

@@ -4732,7 +4732,8 @@ impl Project {
             } else {
                 return Task::ready(Err(anyhow!("worktree not found for symbol")));
             };
-            let symbol_abs_path = worktree_abs_path.join(&symbol.path.path);
+
+            let symbol_abs_path = resolve_path(worktree_abs_path, &symbol.path.path);
             let symbol_uri = if let Ok(uri) = lsp::Url::from_file_path(symbol_abs_path) {
                 uri
             } else {
@@ -6581,7 +6582,14 @@ impl Project {
                 let removed = *change == PathChange::Removed;
                 let abs_path = worktree.absolutize(path);
                 settings_contents.push(async move {
-                    (settings_dir, (!removed).then_some(fs.load(&abs_path).await))
+                    (
+                        settings_dir,
+                        if removed {
+                            None
+                        } else {
+                            Some(async move { fs.load(&abs_path?).await }.await)
+                        },
+                    )
                 });
             }
         }
@@ -8718,6 +8726,20 @@ fn relativize_path(base: &Path, path: &Path) -> PathBuf {
     components.iter().map(|c| c.as_os_str()).collect()
 }
 
+fn resolve_path(base: &Path, path: &Path) -> PathBuf {
+    let mut result = base.to_path_buf();
+    for component in path.components() {
+        match component {
+            Component::ParentDir => {
+                result.pop();
+            }
+            Component::CurDir => (),
+            _ => result.push(component),
+        }
+    }
+    result
+}
+
 impl Item for Buffer {
     fn entry_id(&self, cx: &AppContext) -> Option<ProjectEntryId> {
         File::from_dyn(self.file()).and_then(|file| file.project_entry_id(cx))

crates/project/src/project_tests.rs

@@ -4278,6 +4278,75 @@ fn test_glob_literal_prefix() {
     assert_eq!(glob_literal_prefix("foo/bar/baz.js"), "foo/bar/baz.js");
 }
 
+#[gpui::test]
+async fn test_create_entry(cx: &mut gpui::TestAppContext) {
+    init_test(cx);
+
+    let fs = FakeFs::new(cx.executor().clone());
+    fs.insert_tree(
+        "/one/two",
+        json!({
+            "three": {
+                "a.txt": "",
+                "four": {}
+            },
+            "c.rs": ""
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs.clone(), ["/one/two/three".as_ref()], cx).await;
+    project
+        .update(cx, |project, cx| {
+            let id = project.worktrees().next().unwrap().read(cx).id();
+            project.create_entry((id, "b.."), true, cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+
+    // Can't create paths outside the project
+    let result = project
+        .update(cx, |project, cx| {
+            let id = project.worktrees().next().unwrap().read(cx).id();
+            project.create_entry((id, "../../boop"), true, cx)
+        })
+        .await;
+    assert!(result.is_err());
+
+    // Can't create paths with '..'
+    let result = project
+        .update(cx, |project, cx| {
+            let id = project.worktrees().next().unwrap().read(cx).id();
+            project.create_entry((id, "four/../beep"), true, cx)
+        })
+        .await;
+    assert!(result.is_err());
+
+    assert_eq!(
+        fs.paths(true),
+        vec![
+            PathBuf::from("/"),
+            PathBuf::from("/one"),
+            PathBuf::from("/one/two"),
+            PathBuf::from("/one/two/c.rs"),
+            PathBuf::from("/one/two/three"),
+            PathBuf::from("/one/two/three/a.txt"),
+            PathBuf::from("/one/two/three/b.."),
+            PathBuf::from("/one/two/three/four"),
+        ]
+    );
+
+    // And we cannot open buffers with '..'
+    let result = project
+        .update(cx, |project, cx| {
+            let id = project.worktrees().next().unwrap().read(cx).id();
+            project.open_buffer((id, "../c.rs"), cx)
+        })
+        .await;
+    assert!(result.is_err())
+}
+
 async fn search(
     project: &Model<Project>,
     query: SearchQuery,

crates/project/src/worktree.rs

@@ -965,6 +965,7 @@ impl LocalWorktree {
         let entry = self.refresh_entry(path.clone(), None, cx);
 
         cx.spawn(|this, mut cx| async move {
+            let abs_path = abs_path?;
             let text = fs.load(&abs_path).await?;
             let mut index_task = None;
             let snapshot = this.update(&mut cx, |this, _| this.as_local().unwrap().snapshot())?;
@@ -1050,6 +1051,7 @@ impl LocalWorktree {
 
         cx.spawn(move |this, mut cx| async move {
             let entry = save.await?;
+            let abs_path = abs_path?;
             let this = this.upgrade().context("worktree dropped")?;
 
             let (entry_id, mtime, path) = match entry {
@@ -1139,9 +1141,9 @@ impl LocalWorktree {
         let fs = self.fs.clone();
         let write = cx.background_executor().spawn(async move {
             if is_dir {
-                fs.create_dir(&abs_path).await
+                fs.create_dir(&abs_path?).await
             } else {
-                fs.save(&abs_path, &Default::default(), Default::default())
+                fs.save(&abs_path?, &Default::default(), Default::default())
                     .await
             }
         });
@@ -1188,7 +1190,7 @@ impl LocalWorktree {
         let fs = self.fs.clone();
         let write = cx
             .background_executor()
-            .spawn(async move { fs.save(&abs_path, &text, line_ending).await });
+            .spawn(async move { fs.save(&abs_path?, &text, line_ending).await });
 
         cx.spawn(|this, mut cx| async move {
             write.await?;
@@ -1210,10 +1212,10 @@ impl LocalWorktree {
 
         let delete = cx.background_executor().spawn(async move {
             if entry.is_file() {
-                fs.remove_file(&abs_path, Default::default()).await?;
+                fs.remove_file(&abs_path?, Default::default()).await?;
             } else {
                 fs.remove_dir(
-                    &abs_path,
+                    &abs_path?,
                     RemoveOptions {
                         recursive: true,
                         ignore_if_not_exists: false,
@@ -1252,7 +1254,7 @@ impl LocalWorktree {
         let abs_new_path = self.absolutize(&new_path);
         let fs = self.fs.clone();
         let rename = cx.background_executor().spawn(async move {
-            fs.rename(&abs_old_path, &abs_new_path, Default::default())
+            fs.rename(&abs_old_path?, &abs_new_path?, Default::default())
                 .await
         });
 
@@ -1284,8 +1286,8 @@ impl LocalWorktree {
         let copy = cx.background_executor().spawn(async move {
             copy_recursive(
                 fs.as_ref(),
-                &abs_old_path,
-                &abs_new_path,
+                &abs_old_path?,
+                &abs_new_path?,
                 Default::default(),
             )
             .await
@@ -1609,11 +1611,17 @@ impl Snapshot {
         &self.abs_path
     }
 
-    pub fn absolutize(&self, path: &Path) -> PathBuf {
+    pub fn absolutize(&self, path: &Path) -> Result<PathBuf> {
+        if path
+            .components()
+            .any(|component| !matches!(component, std::path::Component::Normal(_)))
+        {
+            return Err(anyhow!("invalid path"));
+        }
         if path.file_name().is_some() {
-            self.abs_path.join(path)
+            Ok(self.abs_path.join(path))
         } else {
-            self.abs_path.to_path_buf()
+            Ok(self.abs_path.to_path_buf())
         }
     }
 
@@ -2823,7 +2831,7 @@ impl language::LocalFile for File {
         let abs_path = worktree.absolutize(&self.path);
         let fs = worktree.fs.clone();
         cx.background_executor()
-            .spawn(async move { fs.load(&abs_path).await })
+            .spawn(async move { fs.load(&abs_path?).await })
     }
 
     fn buffer_reloaded(

crates/semantic_index/src/semantic_index.rs

@@ -559,7 +559,7 @@ impl SemanticIndex {
                         .spawn(async move {
                             let mut changed_paths = BTreeMap::new();
                             for file in worktree.files(false, 0) {
-                                let absolute_path = worktree.absolutize(&file.path);
+                                let absolute_path = worktree.absolutize(&file.path)?;
 
                                 if file.is_external || file.is_ignored || file.is_symlink {
                                     continue;
@@ -1068,11 +1068,10 @@ impl SemanticIndex {
                                 return true;
                             };
 
-                        worktree_state.changed_paths.retain(|path, info| {
+                        for (path, info) in &worktree_state.changed_paths {
                             if info.is_deleted {
                                 files_to_delete.push((worktree_state.db_id, path.clone()));
-                            } else {
-                                let absolute_path = worktree.read(cx).absolutize(path);
+                            } else if let Ok(absolute_path) = worktree.read(cx).absolutize(path) {
                                 let job_handle = JobHandle::new(pending_file_count_tx);
                                 pending_files.push(PendingFile {
                                     absolute_path,
@@ -1083,9 +1082,8 @@ impl SemanticIndex {
                                     worktree_db_id: worktree_state.db_id,
                                 });
                             }
-
-                            false
-                        });
+                        }
+                        worktree_state.changed_paths.clear();
                         true
                     });