Skip to content

Commit 1fad3d0

Browse files
committed
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.
1 parent 81feced commit 1fad3d0

File tree

1 file changed

+22
-1
lines changed

1 file changed

+22
-1
lines changed

beetsplug/aura.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
from mimetypes import guess_type
1919
import re
20+
import os.path
2021
from os.path import isfile, getsize
2122

2223
from beets.plugins import BeetsPlugin
@@ -595,6 +596,24 @@ def single_resource(self, artist_id):
595596
return self.single_resource_document(artist_resource)
596597

597598

599+
def safe_filename(fn):
600+
"""Check whether a string is a simple (non-path) filename.
601+
602+
For example, `foo.txt` is safe because it is a "plain" filename. But
603+
`foo/bar.txt` and `../foo.txt` and `.` are all non-safe because they
604+
can traverse to other directories other than the current one.
605+
"""
606+
# Rule out any directories.
607+
if os.path.basename(fn) != fn:
608+
return False
609+
610+
# In single names, rule out Unix directory traversal names.
611+
if fn in ('.', '..'):
612+
return False
613+
614+
return True
615+
616+
598617
class ImageDocument(AURADocument):
599618
"""Class for building documents for /images/(id) endpoints."""
600619

@@ -616,6 +635,8 @@ def get_image_path(image_id):
616635
parent_type = id_split[0]
617636
parent_id = id_split[1]
618637
img_filename = "-".join(id_split[2:])
638+
if not safe_filename(img_filename):
639+
return None
619640

620641
# Get the path to the directory parent's images are in
621642
if parent_type == "album":
@@ -631,7 +652,7 @@ def get_image_path(image_id):
631652
# Images for other resource types are not supported
632653
return None
633654

634-
img_path = dir_path + "/" + img_filename
655+
img_path = os.path.join(dir_path, img_filename)
635656
# Check the image actually exists
636657
if isfile(img_path):
637658
return img_path

0 commit comments

Comments
 (0)