From 05c8022db31bdd427055f0eb49c69744b2eb4f8d Mon Sep 17 00:00:00 2001
From: Pascal Vizeli <pvizeli@syshack.ch>
Date: Mon, 21 Oct 2019 12:23:00 +0200
Subject: [PATCH] Check path on extractall (#1336)

* Check path on extractall

* code cleanup

* Add logger

* Fix issue

* Add tests
---
 hassio/addons/addon.py       |  3 ++-
 hassio/snapshots/snapshot.py |  6 +++---
 hassio/utils/tar.py          | 26 +++++++++++++++++++++++---
 tests/utils/test_tarfile.py  | 33 +++++++++++++++++++++++++++++++++
 4 files changed, 61 insertions(+), 7 deletions(-)
 create mode 100644 tests/utils/test_tarfile.py

diff --git a/hassio/addons/addon.py b/hassio/addons/addon.py
index 5b633946e..e47c593d4 100644
--- a/hassio/addons/addon.py
+++ b/hassio/addons/addon.py
@@ -51,6 +51,7 @@ from ..exceptions import (
 )
 from ..utils.apparmor import adjust_profile
 from ..utils.json import read_json_file, write_json_file
+from ..utils.tar import secure_path
 from .model import AddonModel, Data
 from .utils import remove_data
 from .validate import SCHEMA_ADDON_SNAPSHOT, validate_options
@@ -579,7 +580,7 @@ class Addon(AddonModel):
             def _extract_tarfile():
                 """Extract tar snapshot."""
                 with tar_file as snapshot:
-                    snapshot.extractall(path=Path(temp))
+                    snapshot.extractall(path=Path(temp), member=secure_path(snapshot))
 
             try:
                 await self.sys_run_in_executor(_extract_tarfile)
diff --git a/hassio/snapshots/snapshot.py b/hassio/snapshots/snapshot.py
index 9437d748b..5ea4d3d5b 100644
--- a/hassio/snapshots/snapshot.py
+++ b/hassio/snapshots/snapshot.py
@@ -41,7 +41,7 @@ from ..const import (
 from ..coresys import CoreSys, CoreSysAttributes
 from ..exceptions import AddonsError
 from ..utils.json import write_json_file
-from ..utils.tar import SecureTarFile
+from ..utils.tar import SecureTarFile, secure_path
 from .utils import key_to_iv, password_for_validating, password_to_key, remove_folder
 from .validate import ALL_FOLDERS, SCHEMA_SNAPSHOT
 
@@ -248,7 +248,7 @@ class Snapshot(CoreSysAttributes):
         def _extract_snapshot():
             """Extract a snapshot."""
             with tarfile.open(self.tarfile, "r:") as tar:
-                tar.extractall(path=self._tmp.name)
+                tar.extractall(path=self._tmp.name, members=secure_path(tar))
 
         await self.sys_run_in_executor(_extract_snapshot)
 
@@ -396,7 +396,7 @@ class Snapshot(CoreSysAttributes):
             try:
                 _LOGGER.info("Restore folder %s", name)
                 with SecureTarFile(tar_name, "r", key=self._key) as tar_file:
-                    tar_file.extractall(path=origin_dir)
+                    tar_file.extractall(path=origin_dir, members=tar_file)
                 _LOGGER.info("Restore folder %s done", name)
             except (tarfile.TarError, OSError) as err:
                 _LOGGER.warning("Can't restore folder %s: %s", name, err)
diff --git a/hassio/utils/tar.py b/hassio/utils/tar.py
index af3b0a181..9d5be5741 100644
--- a/hassio/utils/tar.py
+++ b/hassio/utils/tar.py
@@ -1,19 +1,22 @@
 """Tarfile fileobject handler for encrypted files."""
 import hashlib
+import logging
 import os
-from pathlib import Path
 import tarfile
-from typing import IO, Optional
+from pathlib import Path
+from typing import IO, Generator, Optional
 
 from cryptography.hazmat.backends import default_backend
 from cryptography.hazmat.primitives import padding
 from cryptography.hazmat.primitives.ciphers import (
-    CipherContext,
     Cipher,
+    CipherContext,
     algorithms,
     modes,
 )
 
+_LOGGER: logging.Logger = logging.getLogger(__name__)
+
 BLOCK_SIZE = 16
 BLOCK_SIZE_BITS = 128
 
@@ -111,3 +114,20 @@ def _generate_iv(key: bytes, salt: bytes) -> bytes:
     for _ in range(100):
         temp_iv = hashlib.sha256(temp_iv).digest()
     return temp_iv[:16]
+
+
+def secure_path(tar: tarfile.TarFile) -> Generator[tarfile.TarInfo, None, None]:
+    """Security safe check of path.
+
+    Prevent ../ or absolut paths
+    """
+    for member in tar:
+        file_path = Path(member.name)
+        try:
+            assert not file_path.is_absolute()
+            Path("/fake", file_path).resolve().relative_to("/fake")
+        except (ValueError, RuntimeError, AssertionError):
+            _LOGGER.warning("Issue with file %s", file_path)
+            continue
+        else:
+            yield member
diff --git a/tests/utils/test_tarfile.py b/tests/utils/test_tarfile.py
new file mode 100644
index 000000000..ed61871ea
--- /dev/null
+++ b/tests/utils/test_tarfile.py
@@ -0,0 +1,33 @@
+"""Test Tarfile functions."""
+
+import attr
+
+from hassio.utils.tar import secure_path
+
+
+@attr.s
+class TarInfo:
+    """Fake TarInfo"""
+
+    name: str = attr.ib()
+
+
+def test_secure_path():
+    """Test Secure Path."""
+    test_list = [
+        TarInfo("test.txt"),
+        TarInfo("data/xy.blob"),
+        TarInfo("bla/blu/ble"),
+        TarInfo("data/../xy.blob"),
+    ]
+    assert test_list == list(secure_path(test_list))
+
+
+def test_not_secure_path():
+    """Test Not secure path."""
+    test_list = [
+        TarInfo("/test.txt"),
+        TarInfo("data/../../xy.blob"),
+        TarInfo("/bla/blu/ble"),
+    ]
+    assert [] == list(secure_path(test_list))