From 0dec6be28a76f2ca73a6000170eeee9daf807f96 Mon Sep 17 00:00:00 2001 From: Quentin Kaiser Date: Tue, 25 Apr 2023 14:59:48 +0200 Subject: [PATCH] fix(handlers): fix edge cases in tar handler (empty name, too long name). tarfile python library does not perform any kind of defensive programming and will trigger OS level errors like ENAMETOOLONG trying to write files that have long names. This commit adds two checks to our safe implementation of tarfile so that we log the issue with a warning and skip the file. Note: tar files with such entries (long name, empty name) do not happen naturally and must be synthesized. That's why decided to skip them, in a similar fashion than GNU tar and 7zip. --- tests/integration/archive/tar/__input__/emptyname.tar | 3 +++ tests/integration/archive/tar/__input__/longname.tar | 3 +++ .../tar/__output__/emptyname.tar_extract/apple.txt | 3 +++ .../tar/__output__/longname.tar_extract/apple.txt | 3 +++ unblob/handlers/archive/_safe_tarfile.py | 9 +++++++++ 5 files changed, 21 insertions(+) create mode 100644 tests/integration/archive/tar/__input__/emptyname.tar create mode 100644 tests/integration/archive/tar/__input__/longname.tar create mode 100644 tests/integration/archive/tar/__output__/emptyname.tar_extract/apple.txt create mode 100644 tests/integration/archive/tar/__output__/longname.tar_extract/apple.txt diff --git a/tests/integration/archive/tar/__input__/emptyname.tar b/tests/integration/archive/tar/__input__/emptyname.tar new file mode 100644 index 0000000000..605a7e19d9 --- /dev/null +++ b/tests/integration/archive/tar/__input__/emptyname.tar @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:8b5233dfb3a4a23a3bf291bfff8ccfb371fbb2136cb093247d30af090d1e4276 +size 10240 diff --git a/tests/integration/archive/tar/__input__/longname.tar b/tests/integration/archive/tar/__input__/longname.tar new file mode 100644 index 0000000000..34bc10a427 --- /dev/null +++ b/tests/integration/archive/tar/__input__/longname.tar @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:bd002ebcec73917b4294602e2999809e4119f5209a74f92fc0f70bff838bca69 +size 10240 diff --git a/tests/integration/archive/tar/__output__/emptyname.tar_extract/apple.txt b/tests/integration/archive/tar/__output__/emptyname.tar_extract/apple.txt new file mode 100644 index 0000000000..fb25b54492 --- /dev/null +++ b/tests/integration/archive/tar/__output__/emptyname.tar_extract/apple.txt @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:303980bcb9e9e6cdec515230791af8b0ab1aaa244b58a8d99152673aa22197d0 +size 6 diff --git a/tests/integration/archive/tar/__output__/longname.tar_extract/apple.txt b/tests/integration/archive/tar/__output__/longname.tar_extract/apple.txt new file mode 100644 index 0000000000..fb25b54492 --- /dev/null +++ b/tests/integration/archive/tar/__output__/longname.tar_extract/apple.txt @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:303980bcb9e9e6cdec515230791af8b0ab1aaa244b58a8d99152673aa22197d0 +size 6 diff --git a/unblob/handlers/archive/_safe_tarfile.py b/unblob/handlers/archive/_safe_tarfile.py index 0c630d0867..13d945660f 100644 --- a/unblob/handlers/archive/_safe_tarfile.py +++ b/unblob/handlers/archive/_safe_tarfile.py @@ -9,6 +9,7 @@ logger = get_logger() RUNNING_AS_ROOT = os.getuid() == 0 +MAX_PATH_LEN = 255 class SafeTarFile(TarFile): @@ -18,6 +19,14 @@ def extract( path_as_path = Path(str(path)) member_name_path = Path(str(member.name)) + if not member.name: + logger.warning("File with empty filename in tar archive. Skipping") + return + + if len(member.name) > MAX_PATH_LEN: + logger.warning("File with filename too long in tar archive. Skipping") + return + if not RUNNING_AS_ROOT and (member.ischr() or member.isblk()): logger.warning( "missing elevated permissions, skipping block and character device creation",