From 1fad3d01aea4627af42a9b7190d6869d2b007cc4 Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 26 Nov 2021 15:35:07 -0500
Subject: [PATCH 1/2] aura: Sanitize filenames in image IDs

When constructing paths to image files to serve, we previously spliced
strings from URL requests directly into the path to be opened. This is
theoretically worrisome because it could allow clients to read other
files that they are not supposed to read.

I'm not actually sure this is a real security problem because Flask's
URL parsing should probably rule out IDs that have `/` in them anyway.
But out of an abundance of caution, this now prevents paths from showing
up in IDs at all---and also prevents `.` and `..` from being valid
names.
---
 beetsplug/aura.py | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/beetsplug/aura.py b/beetsplug/aura.py
index 3799e0df4c..f4ae5527a0 100644
--- a/beetsplug/aura.py
+++ b/beetsplug/aura.py
@@ -17,6 +17,7 @@
 
 from mimetypes import guess_type
 import re
+import os.path
 from os.path import isfile, getsize
 
 from beets.plugins import BeetsPlugin
@@ -595,6 +596,24 @@ def single_resource(self, artist_id):
         return self.single_resource_document(artist_resource)
 
 
+def safe_filename(fn):
+    """Check whether a string is a simple (non-path) filename.
+
+    For example, `foo.txt` is safe because it is a "plain" filename. But
+    `foo/bar.txt` and `../foo.txt` and `.` are all non-safe because they
+    can traverse to other directories other than the current one.
+    """
+    # Rule out any directories.
+    if os.path.basename(fn) != fn:
+        return False
+
+    # In single names, rule out Unix directory traversal names.
+    if fn in ('.', '..'):
+        return False
+
+    return True
+
+
 class ImageDocument(AURADocument):
     """Class for building documents for /images/(id) endpoints."""
 
@@ -616,6 +635,8 @@ def get_image_path(image_id):
         parent_type = id_split[0]
         parent_id = id_split[1]
         img_filename = "-".join(id_split[2:])
+        if not safe_filename(img_filename):
+            return None
 
         # Get the path to the directory parent's images are in
         if parent_type == "album":
@@ -631,7 +652,7 @@ def get_image_path(image_id):
             # Images for other resource types are not supported
             return None
 
-        img_path = dir_path + "/" + img_filename
+        img_path = os.path.join(dir_path, img_filename)
         # Check the image actually exists
         if isfile(img_path):
             return img_path

From 4e692095eb392ad392db246d1726ac5b8058655c Mon Sep 17 00:00:00 2001
From: Adrian Sampson <adrian@radbox.org>
Date: Fri, 26 Nov 2021 15:39:30 -0500
Subject: [PATCH 2/2] Changelog for #4160

---
 docs/changelog.rst | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/docs/changelog.rst b/docs/changelog.rst
index 3bb52baa8f..ee8060b2ea 100644
--- a/docs/changelog.rst
+++ b/docs/changelog.rst
@@ -34,6 +34,11 @@ Other new things:
 
 * Permissions plugin now sets cover art permissions to the file permissions.
 
+Fixes:
+
+* :doc:`/plugins/aura`: Fix a potential security hole when serving image
+  files. :bug:`4160`
+
 1.5.0 (August 19, 2021)
 -----------------------