[PATCH 1/1] archival: new option to disallow path traversals

[Peter Kästle]

Hello busybox community,

did someone have a look at my proposal how to address CVE-2023-39810?
Any feedback would be much appreciated. Thanks.

best regards,
--peter;



On 29.04.24 13:14, Peter Kaestle 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);
> +		}
> +	}
> +#endif
> +
>   	if (archive_handle->ah_flags & ARCHIVE_CREATE_LEADING_DIRS) {
>   		char *slash = strrchr(dst_name, '/');
>   		if (slash) {
> diff --git a/testsuite/cpio.tests b/testsuite/cpio.tests
> index 85e746589..1c0b75297 100755
> --- a/testsuite/cpio.tests
> +++ b/testsuite/cpio.tests
> @@ -154,6 +154,24 @@ testing "cpio -R with extract" \
>   " "" ""
>   SKIP=
>   
> +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 -H newc --create) |
> +(cd cpio.testdir/extract && cpio -vi 2>&1);
> +echo \$?;
> +ls cpio.testdir/dont_write 2>&1" \
> +"\
> +cpio: path traversal detected: ../dont_write
> +1
> +ls: cpio.testdir/dont_write: No such file or directory
> +" "" ""
> +SKIP=
> +
>   # Clean up
>   rm -rf cpio.testdir cpio.testdir2 2>/dev/null
>