Skip to content

Commit

Permalink
Add untar function with guard against path traversal attack and a str…
Browse files Browse the repository at this point in the history
…ip option
  • Loading branch information
friday committed Nov 6, 2022
1 parent 32591c3 commit cc05f11
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 27 deletions.
4 changes: 2 additions & 2 deletions tests/modes/apps/extensions/test_ExtensionDownloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def test_download(self, downloader, mocker, untar, ext_db, download_tarball, dat
assert downloader.download('https://github.com/Ulauncher/ulauncher-timer') == \
'com.github.ulauncher.ulauncher-timer'

untar.assert_called_with(download_tarball.return_value, mock.ANY)
untar.assert_called_with(download_tarball.return_value, mock.ANY, strip=1)
ext_db.save.assert_called_with({'com.github.ulauncher.ulauncher-timer': {
'id': 'com.github.ulauncher.ulauncher-timer',
'url': 'https://github.com/Ulauncher/ulauncher-timer',
Expand Down Expand Up @@ -85,7 +85,7 @@ def test_update(self, downloader, ext_db, gh_ext, download_tarball, untar, datet
assert downloader.update(ext_id)

download_tarball.assert_called_with(gh_ext.get_download_url.return_value)
untar.assert_called_with(download_tarball.return_value, mock.ANY)
untar.assert_called_with(download_tarball.return_value, mock.ANY, strip=1)
ext_db.save.assert_called_with({ext_id: {
'id': ext_id,
'url': 'https://github.com/Ulauncher/ulauncher-timer',
Expand Down
30 changes: 5 additions & 25 deletions ulauncher/modes/extensions/ExtensionDownloader.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import os
import logging
import tarfile
from urllib.request import urlretrieve
from tempfile import mktemp, mkdtemp
from shutil import rmtree, move
from tempfile import mktemp
from shutil import rmtree
from datetime import datetime
from typing import Tuple

from ulauncher.config import PATHS
from ulauncher.utils.decorator.singleton import singleton
from ulauncher.utils.untar import untar
from ulauncher.modes.extensions.ExtensionDb import ExtensionDb, ExtensionRecord
from ulauncher.modes.extensions.ExtensionRemote import ExtensionRemote

Expand Down Expand Up @@ -61,7 +61,7 @@ def download(self, url: str) -> str:

# 3. download & untar
filename = download_tarball(remote.get_download_url(commit_sha))
untar(filename, ext_path)
untar(filename, ext_path, strip=1)

# 4. add to the db
self.ext_db.save({remote.extension_id: {
Expand Down Expand Up @@ -96,7 +96,7 @@ def update(self, ext_id: str) -> bool:

remote = ExtensionRemote(ext.url)
filename = download_tarball(remote.get_download_url(commit_sha))
untar(filename, f"{PATHS.EXTENSIONS}/{ext_id}")
untar(filename, f"{PATHS.EXTENSIONS}/{ext_id}", strip=1)

ext.update(
updated_at=datetime.now().isoformat(),
Expand Down Expand Up @@ -124,26 +124,6 @@ def _find_extension(self, ext_id: str) -> ExtensionRecord:
return ext


def untar(filename: str, ext_path: str) -> None:
"""
1. Remove ext_path
2. Extract tar into temp dir
3. Move contents of <temp_dir>/<project_name>-master/* to ext_path
"""
if os.path.exists(ext_path):
rmtree(ext_path)

temp_ext_path = mkdtemp(prefix='ulauncher_dl_')

with tarfile.open(filename, mode="r") as archive:
archive.extractall(temp_ext_path)

for dir in os.listdir(temp_ext_path):
move(os.path.join(temp_ext_path, dir), ext_path)
# there is only one directory here, so return immediately
return


def download_tarball(url: str) -> str:
dest_tar = mktemp('.tar.gz', prefix='ulauncher_dl_')
filename, _ = urlretrieve(url, dest_tar)
Expand Down
18 changes: 18 additions & 0 deletions ulauncher/utils/untar.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import tarfile
from os.path import exists
from shutil import rmtree


def untar(archive_path: str, output_path: str, overwrite=True, strip=0) -> None:
if overwrite and exists(output_path):
rmtree(output_path)

with tarfile.open(archive_path, mode="r") as archive:
for member in archive.getmembers():
if member.path.startswith("/") or member.path.startswith("../"):
raise Exception("Attempted Path Traversal in tar file")

# Change member paths to strip N levels, like untar --strip-components=N
member.path = member.path.split('/', strip)[-1]

archive.extractall(output_path)

0 comments on commit cc05f11

Please sign in to comment.