m3u: Ignore paths outside the playlist_dir

Fixes #1659

docs/changelog.rst

@@ -25,6 +25,9 @@ Feature release.
 - Core: Define return value of `playlists.delete()` to be a bool, :class:`True`
   on success, :class:`False` otherwise. (PR: :issue:`1702`)
 
+- M3U: Ignore all attempts at accessing files outside the
+  :confval:`m3u/playlist_dir`. (Partly fixes: :issue:`1659`, PR: :issue:`1702`)
+
 - File: Change default ordering to show directories first, then files. (PR:
   :issue:`1595`)
 

mopidy/m3u/playlists.py

@@ -9,6 +9,7 @@ import os
 import tempfile
 
 from mopidy import backend
+from mopidy.internal import path
 
 from . import Extension, translator
 
@@ -89,6 +90,9 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
 
     def delete(self, uri):
         path = translator.uri_to_path(uri)
+        if not self._is_in_basedir(path):
+            logger.debug('Ignoring path outside playlist dir: %s', uri)
+            return False
         try:
             os.remove(self._abspath(path))
         except EnvironmentError as e:
@@ -99,6 +103,9 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
 
     def get_items(self, uri):
         path = translator.uri_to_path(uri)
+        if not self._is_in_basedir(path):
+            logger.debug('Ignoring path outside playlist dir: %s', uri)
+            return None
         try:
             with self._open(path, 'r') as fp:
                 items = translator.load_items(fp, self._base_dir)
@@ -109,6 +116,9 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
 
     def lookup(self, uri):
         path = translator.uri_to_path(uri)
+        if not self._is_in_basedir(path):
+            logger.debug('Ignoring path outside playlist dir: %s', uri)
+            return None
         try:
             with self._open(path, 'r') as fp:
                 items = translator.load_items(fp, self._base_dir)
@@ -123,6 +133,10 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
 
     def save(self, playlist):
         path = translator.uri_to_path(playlist.uri)
+        if not self._is_in_basedir(path):
+            logger.debug(
+                'Ignoring path outside playlist dir: %s', playlist.uri)
+            return None
         name = translator.name_from_path(path)
         try:
             with self._open(path, 'w') as fp:
@@ -140,6 +154,11 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
     def _abspath(self, path):
         return os.path.join(self._playlists_dir, path)
 
+    def _is_in_basedir(self, local_path):
+        if not os.path.isabs(local_path):
+            local_path = os.path.join(self._playlists_dir, local_path)
+        return path.is_path_inside_base_dir(local_path, self._playlists_dir)
+
     def _open(self, path, mode='r'):
         if path.endswith(b'.m3u8'):
             encoding = 'utf-8'
@@ -147,6 +166,10 @@ class M3UPlaylistsProvider(backend.PlaylistsProvider):
             encoding = self._default_encoding
         if not os.path.isabs(path):
             path = os.path.join(self._playlists_dir, path)
+        if not self._is_in_basedir(path):
+            raise Exception(
+                'Path (%s) is not inside playlist_dir (%s)'
+                % (path, self._playlists_dir))
         if 'w' in mode:
             return replace(path, mode, encoding=encoding, errors='replace')
         else:

tests/m3u/test_playlists.py

@@ -98,6 +98,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase):
         self.assertTrue(success)
         self.assertFalse(os.path.exists(path))
 
+    def test_delete_on_path_outside_playlist_dir_returns_none(self):
+        success = self.core.playlists.delete('m3u:///etc/passwd')
+
+        self.assertFalse(success)
+
     def test_playlist_contents_is_written_to_disk(self):
         track = Track(uri=generate_song(1))
         playlist = self.core.playlists.create('test')
@@ -217,6 +222,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase):
 
         self.assertEqual(original_playlist, looked_up_playlist)
 
+    def test_lookup_on_path_outside_playlist_dir_returns_none(self):
+        result = self.core.playlists.lookup('m3u:///etc/passwd')
+
+        self.assertIsNone(result)
+
     def test_refresh(self):
         playlist = self.core.playlists.create('test')
         self.assertEqual(playlist, self.core.playlists.lookup(playlist.uri))
@@ -252,6 +262,10 @@ class M3UPlaylistsProviderTest(unittest.TestCase):
         path = os.path.join(self.playlists_dir, b'test.m3u')
         self.assertTrue(os.path.exists(path))
 
+    def test_save_on_path_outside_playlist_dir_returns_none(self):
+        result = self.core.playlists.save(Playlist(uri='m3u:///tmp/test.m3u'))
+        self.assertIsNone(result)
+
     def test_playlist_with_unknown_track(self):
         track = Track(uri='file:///dev/null')
         playlist = self.core.playlists.create('test')
@@ -331,6 +345,11 @@ class M3UPlaylistsProviderTest(unittest.TestCase):
 
         self.assertIsNone(item_refs)
 
+    def test_get_items_from_file_outside_playlist_dir_returns_none(self):
+        item_refs = self.core.playlists.get_items('m3u:///etc/passwd')
+
+        self.assertIsNone(item_refs)
+
 
 class M3UPlaylistsProviderBaseDirectoryTest(M3UPlaylistsProviderTest):