[RESEND(4) PATCH] archival: disallow path traversals (CVE-2023-39810)

[Denys Vlasenko]

On Wed, Oct 2, 2024 at 10:12 AM Peter Kaestle <peter.kaestle at nokia.com> wrote:
>
> Create new configure option for archival/libarchive based extractions to
> disallow path traversals.
> As this is a paranoid option and might introduce backward
> incompatibiltiy, default it to no.
>
> Fixes: CVE-2023-39810
>
> Signed-off-by: Peter Kaestle <peter.kaestle at nokia.com>
> Reviewed-by: Samuel Sapalski <samuel.sapalski at nokia.com>
> ---
>  archival/Config.src                    |  7 +++++++
>  archival/libarchive/data_extract_all.c | 22 ++++++++++++++++++++++
>  testsuite/cpio.tests                   | 18 ++++++++++++++++++
>  3 files changed, 47 insertions(+)
>
> diff --git a/archival/Config.src b/archival/Config.src
> index 6f4f30c43..ac9d3db95 100644
> --- a/archival/Config.src
> +++ b/archival/Config.src
> @@ -35,4 +35,11 @@ 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 "enable path traversal protection"
> +       default n
> +       help
> +       This option will disallow extraction of files outside of the
> +       destination directory.
> +
>  endmenu
> diff --git a/archival/libarchive/data_extract_all.c b/archival/libarchive/data_extract_all.c
> index 049c2c156..cb5d5c4ca 100644
> --- a/archival/libarchive/data_extract_all.c
> +++ b/archival/libarchive/data_extract_all.c
> @@ -66,6 +66,28 @@ void FAST_FUNC data_extract_all(archive_handle_t *archive_handle)
>         }
>  #endif
>
> +#if ENABLE_FEATURE_PATH_TRAVERSAL_PROTECTION
> +       if (strstr(dst_name, "../")) {
> +               char *resolved_dst_path, *cwd;
> +
> +               cwd = getcwd(NULL, 0);
> +
> +               resolved_dst_path = xmalloc_realpath_coreutils(dst_name);
> +               if (resolved_dst_path) {
> +                       if (strncmp(cwd, resolved_dst_path, strlen(cwd))) {
> +                               errno = 0; /* suppress missleading error prints */
> +                               free(resolved_dst_path);
> +                               bb_perror_msg_and_die("path traversal detected: %s",
> +                                               dst_name);
> +                       }
> +                       free(resolved_dst_path);
> +               } else {
> +                       bb_perror_msg_and_die("cannot allocate memory for real path: %s",
> +                                       dst_name);
> +               }
> +       }

Forgot to "free(cwd)".

The test does not protect against absolute paths. Think dst_name =
"/etc/shadow".

In general, tar already has strip_unsafe_prefix() mechanism to protect
against this.
I'll go with a solution to just call

         dst_name = (char *)strip_unsafe_prefix(dst_name);

here instead.