From 6051ee4cb1607bb7c485096f2bb78ed92f53d81a Mon Sep 17 00:00:00 2001
From: Mike Welsh <mwelsh@gmail.com>
Date: Sun, 9 May 2021 22:06:36 -0700
Subject: [PATCH] core: Disallow .. in shared object paths (fix #3961)

Toss out any shared objects that contain ".." in the name
to avoid accessing files outside of the Ruffle data directory.

The DiskStorageBackend also will fail any requests with a ".."
component as an extra precaution.

Fixes #3961.
---
 core/src/avm1/globals/shared_object.rs | 12 ++++++++++-
 desktop/src/storage.rs                 | 29 ++++++++++++++++++--------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/core/src/avm1/globals/shared_object.rs b/core/src/avm1/globals/shared_object.rs
index 3ff30a8a4..0a863c018 100644
--- a/core/src/avm1/globals/shared_object.rs
+++ b/core/src/avm1/globals/shared_object.rs
@@ -378,7 +378,17 @@ pub fn get_local<'gc>(
     };
 
     // Final SO path: foo.com/folder/game.swf/SOName
-    let full_name = format!("{}/{}/{}", movie_host, local_path, name);
+    // SOName may be a path containing slashes. In this case, prefix with # to mimic Flash Player behavior.
+    let prefix = if name.contains('/') { "#" } else { "" };
+    let full_name = format!("{}/{}/{}{}", movie_host, local_path, prefix, name);
+
+    // Avoid any paths with `..` to prevent SWFs from crawling the file system on desktop.
+    // Flash will generally fail to save shared objects with a path component starting with `.`,
+    // so let's disallow them altogether.
+    if full_name.split('/').any(|s| s.starts_with('.')) {
+        log::error!("SharedObject.get_local: Invalid path with .. segments");
+        return Ok(Value::Null);
+    }
 
     // Check if this is referencing an existing shared object
     if let Some(so) = activation.context.shared_objects.get(&full_name) {
diff --git a/desktop/src/storage.rs b/desktop/src/storage.rs
index 0480c92c8..c7442b10f 100644
--- a/desktop/src/storage.rs
+++ b/desktop/src/storage.rs
@@ -2,7 +2,7 @@ use ruffle_core::backend::storage::StorageBackend;
 use std::fs;
 use std::fs::File;
 use std::io::Write;
-use std::path::PathBuf;
+use std::path::{Component, Path, PathBuf};
 
 pub struct DiskStorageBackend {
     base_path: PathBuf,
@@ -28,6 +28,11 @@ impl DiskStorageBackend {
         }
     }
 
+    /// Verifies that the path contains no `..` components to prevent accessing files outside of the Ruffle directory.
+    fn is_path_allowed(path: &Path) -> bool {
+        path.components().all(|c| c != Component::ParentDir)
+    }
+
     fn get_shared_object_path(&self, name: &str) -> PathBuf {
         self.shared_objects_path.join(format!("{}.sol", name))
     }
@@ -43,12 +48,12 @@ impl DiskStorageBackend {
 impl StorageBackend for DiskStorageBackend {
     fn get(&self, name: &str) -> Option<Vec<u8>> {
         let path = self.get_shared_object_path(name);
+        if !Self::is_path_allowed(&path) {
+            return None;
+        }
         match std::fs::read(path) {
             Ok(data) => Some(data),
             Err(e) if e.kind() == std::io::ErrorKind::NotFound => {
-                // Backwards compatibility with pre-05/09/2021:
-                // Search for data in old location, without .sol extension.
-                // Remove this eventually.
                 let path = self.get_back_compat_shared_object_path(name);
                 match std::fs::read(path) {
                     Ok(data) => Some(data),
@@ -66,8 +71,11 @@ impl StorageBackend for DiskStorageBackend {
     }
 
     fn put(&mut self, name: &str, value: &[u8]) -> bool {
-        let full_path = self.get_shared_object_path(name);
-        if let Some(parent_dir) = full_path.parent() {
+        let path = self.get_shared_object_path(name);
+        if !Self::is_path_allowed(&path) {
+            return false;
+        }
+        if let Some(parent_dir) = path.parent() {
             if !parent_dir.exists() {
                 if let Err(r) = fs::create_dir_all(&parent_dir) {
                     log::warn!("Unable to create storage dir {}", r);
@@ -76,7 +84,7 @@ impl StorageBackend for DiskStorageBackend {
             }
         }
 
-        match File::create(full_path) {
+        match File::create(path) {
             Ok(mut file) => {
                 if let Err(r) = file.write_all(&value) {
                     log::warn!("Unable to write file content {:?}", r);
@@ -93,7 +101,10 @@ impl StorageBackend for DiskStorageBackend {
     }
 
     fn remove_key(&mut self, name: &str) {
-        let full_path = self.get_shared_object_path(name);
-        let _ = fs::remove_file(full_path);
+        let path = self.get_shared_object_path(name);
+        if !Self::is_path_allowed(&path) {
+            return;
+        }
+        let _ = fs::remove_file(path);
     }
 }