From 9a8796436b9b0641e13480811902ea2ac57881d3 Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Wed, 2 Oct 2024 10:12:05 +0200 Subject: [PATCH] archival: disallow path traversals (CVE-2023-39810) Create new configure option for archival/libarchive based extractions to disallow path traversals. As this is a paranoid option and might introduce backward incompatibility, default it to no. Fixes: CVE-2023-39810 Based on the patch by Peter Kaestle function old new delta data_extract_all 921 945 +24 strip_unsafe_prefix 101 102 +1 ------------------------------------------------------------------------------ (add/remove: 0/0 grow/shrink: 2/0 up/down: 25/0) Total: 25 bytes Signed-off-by: Denys Vlasenko CVE: CVE-2023-39810 Upstream-Status: Backport [https://git.busybox.net/busybox/commit/?id=9a8796436b9b0641e13480811902ea2ac57881d3] Signed-off-by: Peter Marko --- archival/Config.src | 11 +++++++++++ archival/libarchive/data_extract_all.c | 8 ++++++++ archival/libarchive/unsafe_prefix.c | 6 +++++- scripts/kconfig/lxdialog/check-lxdialog.sh | 2 +- testsuite/cpio.tests | 23 ++++++++++++++++++++++ 5 files changed, 48 insertions(+), 2 deletions(-) diff --git a/archival/Config.src b/archival/Config.src index 6f4f30c43..cbcd7217c 100644 --- a/archival/Config.src +++ b/archival/Config.src @@ -35,4 +35,15 @@ config FEATURE_LZMA_FAST This option reduces decompression time by about 25% at the cost of a 1K bigger binary. +config FEATURE_PATH_TRAVERSAL_PROTECTION + bool "Prevent extraction of filenames with /../ path component" + default n + help + busybox tar and unzip remove "PREFIX/../" (if it exists) + from extracted names. + This option enables this behavior for all other unpacking applets, + such as cpio, ar, rpm. + GNU cpio 2.15 has NO such sanity check. +# try other archivers and document their behavior? + endmenu diff --git a/archival/libarchive/data_extract_all.c b/archival/libarchive/data_extract_all.c index 049c2c156..8a69711c1 100644 --- a/archival/libarchive/data_extract_all.c +++ b/archival/libarchive/data_extract_all.c @@ -65,6 +65,14 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle) } while (--n != 0); } #endif +#if ENABLE_FEATURE_PATH_TRAVERSAL_PROTECTION + /* Strip leading "/" and up to last "/../" path component */ + dst_name = (char *)strip_unsafe_prefix(dst_name); +#endif +// ^^^ This may be a problem if some applets do need to extract absolute names. +// (Probably will need to invent ARCHIVE_ALLOW_UNSAFE_NAME flag). +// You might think that rpm needs it, but in my tests rpm's internal cpio +// archive has names like "./usr/bin/FOO", not "/usr/bin/FOO". if (archive_handle->ah_flags & ARCHIVE_CREATE_LEADING_DIRS) { char *slash = strrchr(dst_name, '/'); diff --git a/archival/libarchive/unsafe_prefix.c b/archival/libarchive/unsafe_prefix.c index 33e487bf9..667081195 100644 --- a/archival/libarchive/unsafe_prefix.c +++ b/archival/libarchive/unsafe_prefix.c @@ -14,7 +14,11 @@ const char* FAST_FUNC strip_unsafe_prefix(const char *str) cp++; continue; } - if (is_prefixed_with(cp, "/../"+1)) { + /* We are called lots of times. + * is_prefixed_with(cp, "../") is slower than open-coding it, + * with minimal code growth (~few bytes). + */ + if (cp[0] == '.' && cp[1] == '.' && cp[2] == '/') { cp += 3; continue; } diff --git a/scripts/kconfig/lxdialog/check-lxdialog.sh b/scripts/kconfig/lxdialog/check-lxdialog.sh index 5075ebf2d..910ca1f7c 100755 --- a/scripts/kconfig/lxdialog/check-lxdialog.sh +++ b/scripts/kconfig/lxdialog/check-lxdialog.sh @@ -55,7 +55,7 @@ trap "rm -f $tmp" 0 1 2 3 15 check() { $cc -x c - -o $tmp 2>/dev/null <<'EOF' #include CURSES_LOC -main() {} +int main() { return 0; } EOF if [ $? != 0 ]; then echo " *** Unable to find the ncurses libraries or the" 1>&2 diff --git a/testsuite/cpio.tests b/testsuite/cpio.tests index 85e746589..a4462c53e 100755 --- a/testsuite/cpio.tests +++ b/testsuite/cpio.tests @@ -154,6 +154,29 @@ testing "cpio -R with extract" \ " "" "" SKIP= +# Create an archive containing a file with "../dont_write" filename. +# See that it will not be allowed to unpack. +# NB: GNU cpio 2.15 DOES NOT do such checks. +optional FEATURE_PATH_TRAVERSAL_PROTECTION +rm -rf cpio.testdir +mkdir -p cpio.testdir/prepare/inner +echo "file outside of destination was written" > cpio.testdir/prepare/dont_write +echo "data" > cpio.testdir/prepare/inner/to_extract +mkdir -p cpio.testdir/extract +testing "cpio extract file outside of destination" "\ +(cd cpio.testdir/prepare/inner && echo -e '../dont_write\nto_extract' | cpio -o -H newc) | (cd cpio.testdir/extract && cpio -vi 2>&1) +echo \$? +ls cpio.testdir/dont_write 2>&1" \ +"\ +cpio: removing leading '../' from member names +../dont_write +to_extract +1 blocks +0 +ls: cpio.testdir/dont_write: No such file or directory +" "" "" +SKIP= + # Clean up rm -rf cpio.testdir cpio.testdir2 2>/dev/null