From 3ccc85ca41a2526b4e19b6d999c90324b1bb4abb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Tue, 12 May 2026 04:16:02 +0200 Subject: [PATCH 1/5] Validate tarball paths on create --- src/hex_tarball.erl | 116 +++++++++++++++++++++++++++++-------- test/hex_tarball_SUITE.erl | 23 ++++++-- 2 files changed, 110 insertions(+), 29 deletions(-) diff --git a/src/hex_tarball.erl b/src/hex_tarball.erl index c1aded01..6f1c093a 100644 --- a/src/hex_tarball.erl +++ b/src/hex_tarball.erl @@ -74,35 +74,40 @@ create(Metadata, Files, Config) -> false -> {error, {tarball, {file_too_big, "metadata.config"}}}; true -> - ContentsTarball = create_memory_tarball(Files), - ContentsTarballCompressed = gzip(ContentsTarball), - InnerChecksum = inner_checksum(?VERSION, MetadataBinary, ContentsTarballCompressed), - InnerChecksumBase16 = encode_base16(InnerChecksum), - - OuterFiles = [ - {"VERSION", ?VERSION}, - {"CHECKSUM", InnerChecksumBase16}, - {"metadata.config", MetadataBinary}, - {"contents.tar.gz", ContentsTarballCompressed} - ], - - case valid_size(ContentsTarball, TarballMaxUncompressedSize) of - true -> - Tarball = create_memory_tarball(OuterFiles), - OuterChecksum = checksum(Tarball), + case validate_create_files(Files) of + ok -> + ContentsTarball = create_memory_tarball(Files), + ContentsTarballCompressed = gzip(ContentsTarball), + InnerChecksum = inner_checksum(?VERSION, MetadataBinary, ContentsTarballCompressed), + InnerChecksumBase16 = encode_base16(InnerChecksum), + + OuterFiles = [ + {"VERSION", ?VERSION}, + {"CHECKSUM", InnerChecksumBase16}, + {"metadata.config", MetadataBinary}, + {"contents.tar.gz", ContentsTarballCompressed} + ], - case valid_size(Tarball, TarballMaxSize) of + case valid_size(ContentsTarball, TarballMaxUncompressedSize) of true -> - {ok, #{ - tarball => Tarball, - outer_checksum => OuterChecksum, - inner_checksum => InnerChecksum - }}; + Tarball = create_memory_tarball(OuterFiles), + OuterChecksum = checksum(Tarball), + + case valid_size(Tarball, TarballMaxSize) of + true -> + {ok, #{ + tarball => Tarball, + outer_checksum => OuterChecksum, + inner_checksum => InnerChecksum + }}; + false -> + {error, {tarball, {too_big_compressed, TarballMaxSize}}} + end; false -> - {error, {tarball, {too_big_compressed, TarballMaxSize}}} + {error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}} end; - false -> - {error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}} + {error, _} = Error -> + Error end end. @@ -344,6 +349,10 @@ format_error({tarball, {bad_version, Vsn}}) -> io_lib:format("unsupported version: ~p", [Vsn]); format_error({tarball, invalid_checksum}) -> "invalid tarball checksum"; +format_error({tarball, {unsafe_path, Name}}) -> + io_lib:format("unsafe path in tarball: ~s", [Name]); +format_error({tarball, {unsafe_symlink, Name, LinkTarget}}) -> + io_lib:format("unsafe symlink in tarball: ~s -> ~s", [Name, LinkTarget]); format_error({tarball, Reason}) -> "tarball error, " ++ hex_erl_tar:format_error(Reason); format_error({inner_tarball, Reason}) -> @@ -881,6 +890,63 @@ guess_build_tools(Metadata) -> %% Tar Helpers %%==================================================================== +%% @private +validate_create_files(Files) when is_list(Files) -> + validate_create_files(Files, ok). + +validate_create_files(_Files, {error, _} = Error) -> + Error; +validate_create_files([], ok) -> + ok; +validate_create_files([File | Rest], ok) -> + validate_create_files(Rest, validate_create_file(File)). + +validate_create_file({Filename, Contents}) when is_list(Filename), is_binary(Contents) -> + validate_archive_path(Filename); +validate_create_file(Filename) when is_list(Filename) -> + validate_create_file({Filename, Filename}); +validate_create_file({Filename, AbsFilename}) when is_list(Filename), is_list(AbsFilename) -> + case validate_archive_path(Filename) of + ok -> validate_symlink_target(Filename, AbsFilename); + {error, _} = Error -> Error + end. + +validate_archive_path(Filename) -> + case safe_relative_archive_path(Filename) of + false -> {error, {tarball, {unsafe_path, Filename}}}; + true -> ok + end. + +validate_symlink_target(ArchiveName, SourcePath) -> + case file:read_link_info(SourcePath, []) of + {ok, #file_info{type = symlink}} -> + {ok, LinkTarget} = file:read_link(SourcePath), + ResolvedTarget = filename:join(filename:dirname(ArchiveName), LinkTarget), + case safe_relative_archive_path(ResolvedTarget) of + false -> {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}}; + true -> ok + end; + _ -> + ok + end. + +safe_relative_archive_path(Path) -> + case filename:pathtype(Path) of + relative -> safe_relative_archive_path(filename:split(Path), []); + _ -> false + end. + +safe_relative_archive_path([], _Acc) -> + true; +safe_relative_archive_path(["." | Rest], Acc) -> + safe_relative_archive_path(Rest, Acc); +safe_relative_archive_path([".." | _Rest], []) -> + false; +safe_relative_archive_path([".." | Rest], [_ | Acc]) -> + safe_relative_archive_path(Rest, Acc); +safe_relative_archive_path([_Part | Rest], Acc) -> + safe_relative_archive_path(Rest, [ok | Acc]). + %% @private unpack_tarball(Source, memory, MaxSize) -> case hex_erl_tar:extract(Source, [memory, compressed, {max_size, MaxSize}]) of diff --git a/test/hex_tarball_SUITE.erl b/test/hex_tarball_SUITE.erl index b70548a3..4ebd1316 100644 --- a/test/hex_tarball_SUITE.erl +++ b/test/hex_tarball_SUITE.erl @@ -12,6 +12,7 @@ all() -> timestamps_and_permissions_test, symlinks_test, symlinks_parent_dir_test, + unsafe_paths_to_create_test, memory_test, build_tools_test, requirements_test, @@ -267,10 +268,24 @@ symlinks_parent_dir_test(Config) -> ok = file:make_symlink("../../escape", UnsafeLink), UnsafeFiles = [{"dir/link.sh", UnsafeLink}], - {ok, #{tarball := UnsafeTarball}} = hex_tarball:create(Metadata, UnsafeFiles), - UnpackDir3 = filename:join(BaseDir, "symlinks_parent_dir3"), - {error, {inner_tarball, {"../../escape", unsafe_symlink}}} = - hex_tarball:unpack(UnsafeTarball, UnpackDir3), + {error, {tarball, {unsafe_symlink, "dir/link.sh", "../../escape"}}} = + hex_tarball:create(Metadata, UnsafeFiles), + + ok. + +unsafe_paths_to_create_test(Config) -> + BaseDir = ?config(priv_dir, Config), + Metadata = #{<<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>}, + + {error, {tarball, {unsafe_path, "../README.md"}}} = + hex_tarball:create(Metadata, [{"../README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "/README.md"}}} = + hex_tarball:create(Metadata, [{"/README.md", <<"README">>}]), + + UnsafeLink = filename:join(BaseDir, "unsafe_link"), + ok = file:make_symlink("../../README.md", UnsafeLink), + {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = + hex_tarball:create(Metadata, [{"README.md", UnsafeLink}]), ok. From f82b94367b6c51e2c85bd41d1c88d3f1a9af40a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Tue, 12 May 2026 04:23:27 +0200 Subject: [PATCH 2/5] Expand tarball create validation --- src/hex_tarball.erl | 35 +++++++++++++++++++++++------------ test/hex_tarball_SUITE.erl | 29 +++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/src/hex_tarball.erl b/src/hex_tarball.erl index 6f1c093a..5977d71c 100644 --- a/src/hex_tarball.erl +++ b/src/hex_tarball.erl @@ -139,20 +139,25 @@ create_docs(Files, Config) -> docs_tarball_max_uncompressed_size := TarballMaxUncompressedSize } = Config, - UncompressedTarball = create_memory_tarball(Files), - - case valid_size(UncompressedTarball, TarballMaxUncompressedSize) of - true -> - Tarball = gzip(UncompressedTarball), + case validate_create_files(Files) of + ok -> + UncompressedTarball = create_memory_tarball(Files), - case valid_size(Tarball, TarballMaxSize) of + case valid_size(UncompressedTarball, TarballMaxUncompressedSize) of true -> - {ok, Tarball}; + Tarball = gzip(UncompressedTarball), + + case valid_size(Tarball, TarballMaxSize) of + true -> + {ok, Tarball}; + false -> + {error, {tarball, {too_big_compressed, TarballMaxSize}}} + end; false -> - {error, {tarball, {too_big_compressed, TarballMaxSize}}} + {error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}} end; - false -> - {error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}} + {error, _} = Error -> + Error end. -spec create_docs(files()) -> {ok, tarball()} | {error, term()}. @@ -353,6 +358,8 @@ format_error({tarball, {unsafe_path, Name}}) -> io_lib:format("unsafe path in tarball: ~s", [Name]); format_error({tarball, {unsafe_symlink, Name, LinkTarget}}) -> io_lib:format("unsafe symlink in tarball: ~s -> ~s", [Name, LinkTarget]); +format_error({tarball, {unsupported_file_type, Name, Type}}) -> + io_lib:format("unsupported file type in tarball: ~s (~p)", [Name, Type]); format_error({tarball, Reason}) -> "tarball error, " ++ hex_erl_tar:format_error(Reason); format_error({inner_tarball, Reason}) -> @@ -907,7 +914,7 @@ validate_create_file(Filename) when is_list(Filename) -> validate_create_file({Filename, Filename}); validate_create_file({Filename, AbsFilename}) when is_list(Filename), is_list(AbsFilename) -> case validate_archive_path(Filename) of - ok -> validate_symlink_target(Filename, AbsFilename); + ok -> validate_source_file(Filename, AbsFilename); {error, _} = Error -> Error end. @@ -917,8 +924,10 @@ validate_archive_path(Filename) -> true -> ok end. -validate_symlink_target(ArchiveName, SourcePath) -> +validate_source_file(ArchiveName, SourcePath) -> case file:read_link_info(SourcePath, []) of + {ok, #file_info{type = Type}} when Type =:= regular; Type =:= directory -> + ok; {ok, #file_info{type = symlink}} -> {ok, LinkTarget} = file:read_link(SourcePath), ResolvedTarget = filename:join(filename:dirname(ArchiveName), LinkTarget), @@ -926,6 +935,8 @@ validate_symlink_target(ArchiveName, SourcePath) -> false -> {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}}; true -> ok end; + {ok, #file_info{type = Type}} -> + {error, {tarball, {unsupported_file_type, ArchiveName, Type}}}; _ -> ok end. diff --git a/test/hex_tarball_SUITE.erl b/test/hex_tarball_SUITE.erl index 4ebd1316..f4b02b03 100644 --- a/test/hex_tarball_SUITE.erl +++ b/test/hex_tarball_SUITE.erl @@ -13,6 +13,7 @@ all() -> symlinks_test, symlinks_parent_dir_test, unsafe_paths_to_create_test, + unsupported_file_types_to_create_test, memory_test, build_tools_test, requirements_test, @@ -281,14 +282,39 @@ unsafe_paths_to_create_test(Config) -> hex_tarball:create(Metadata, [{"../README.md", <<"README">>}]), {error, {tarball, {unsafe_path, "/README.md"}}} = hex_tarball:create(Metadata, [{"/README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "../README.md"}}} = + hex_tarball:create_docs([{"../README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "/README.md"}}} = + hex_tarball:create_docs([{"/README.md", <<"README">>}]), UnsafeLink = filename:join(BaseDir, "unsafe_link"), ok = file:make_symlink("../../README.md", UnsafeLink), {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = hex_tarball:create(Metadata, [{"README.md", UnsafeLink}]), + {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = + hex_tarball:create_docs([{"README.md", UnsafeLink}]), ok. +unsupported_file_types_to_create_test(Config) -> + case os:find_executable("mkfifo") of + false -> + {skip, "mkfifo not available"}; + Mkfifo -> + BaseDir = ?config(priv_dir, Config), + Metadata = #{<<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>}, + Fifo = filename:join(BaseDir, "fifo"), + + _ = file:delete(Fifo), + [] = os:cmd(Mkfifo ++ " " ++ shell_quote(Fifo)), + {error, {tarball, {unsupported_file_type, "fifo", other}}} = + hex_tarball:create(Metadata, [{"fifo", Fifo}]), + {error, {tarball, {unsupported_file_type, "fifo", other}}} = + hex_tarball:create_docs([{"fifo", Fifo}]), + + ok + end. + build_tools_test(_Config) -> Metadata = #{<<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>}, Contents = [], @@ -910,6 +936,9 @@ epoch() -> Y2kEpoch = calendar:datetime_to_gregorian_seconds({{2000, 1, 1}, {0, 0, 0}}), Y2kEpoch - NixEpoch. +shell_quote(String) -> + "'" ++ lists:flatten(string:replace(String, "'", "'\\''", all)) ++ "'". + unpack_files(Files) -> FileList = maps:to_list(Files), ok = hex_erl_tar:create("test.tar", FileList, [write]), From 9087811d8e626471063aae92e3bf53539bf923a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Tue, 12 May 2026 04:46:53 +0200 Subject: [PATCH 3/5] Format tarball validation --- src/hex_tarball.erl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/hex_tarball.erl b/src/hex_tarball.erl index 5977d71c..4aa93195 100644 --- a/src/hex_tarball.erl +++ b/src/hex_tarball.erl @@ -78,7 +78,9 @@ create(Metadata, Files, Config) -> ok -> ContentsTarball = create_memory_tarball(Files), ContentsTarballCompressed = gzip(ContentsTarball), - InnerChecksum = inner_checksum(?VERSION, MetadataBinary, ContentsTarballCompressed), + InnerChecksum = inner_checksum( + ?VERSION, MetadataBinary, ContentsTarballCompressed + ), InnerChecksumBase16 = encode_base16(InnerChecksum), OuterFiles = [ From 032daa04ff7c779fc8596d4e98ea9f7448672fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Wed, 13 May 2026 01:04:46 +0200 Subject: [PATCH 4/5] Validate tarball sources within root --- src/hex_core.erl | 5 ++ src/hex_tarball.erl | 93 ++++++++++++++++++++++++++++++-------- test/hex_tarball_SUITE.erl | 27 +++++++++++ 3 files changed, 106 insertions(+), 19 deletions(-) diff --git a/src/hex_core.erl b/src/hex_core.erl index c5f21052..83c3b3ab 100644 --- a/src/hex_core.erl +++ b/src/hex_core.erl @@ -66,6 +66,9 @@ %% * `tarball_max_uncompressed_size' - Maximum size of uncompressed package tarball, defaults to %% `134_217_728' (128 MiB). Set to `infinity' to not enforce the limit. %% +%% * `tarball_files_root' - Root directory source files must resolve inside when creating tarballs. +%% Set to `undefined' to skip source root validation (default: `undefined'). +%% %% * `docs_tarball_max_size' - Maximum size of docs tarball, defaults to %% `16_777_216' (16 MiB). Set to `infinity' to not enforce the limit. %% @@ -113,6 +116,7 @@ repo_verify => boolean(), repo_verify_origin => boolean(), send_100_continue => boolean(), + tarball_files_root => file:filename() | undefined, tarball_max_size => pos_integer() | infinity, tarball_max_uncompressed_size => pos_integer() | infinity, docs_tarball_max_size => pos_integer() | infinity, @@ -140,6 +144,7 @@ default_config() -> repo_verify => true, repo_verify_origin => true, send_100_continue => true, + tarball_files_root => undefined, tarball_max_size => 16 * 1024 * 1024, tarball_max_uncompressed_size => 128 * 1024 * 1024, docs_tarball_max_size => 16 * 1024 * 1024, diff --git a/src/hex_tarball.erl b/src/hex_tarball.erl index 4aa93195..3d54a96e 100644 --- a/src/hex_tarball.erl +++ b/src/hex_tarball.erl @@ -67,6 +67,7 @@ create(Metadata, Files, Config) -> tarball_max_size := TarballMaxSize, tarball_max_uncompressed_size := TarballMaxUncompressedSize } = Config, + FilesRoot = maps:get(tarball_files_root, Config, undefined), MetadataBinary = encode_metadata(Metadata), @@ -74,7 +75,7 @@ create(Metadata, Files, Config) -> false -> {error, {tarball, {file_too_big, "metadata.config"}}}; true -> - case validate_create_files(Files) of + case validate_create_files(Files, FilesRoot) of ok -> ContentsTarball = create_memory_tarball(Files), ContentsTarballCompressed = gzip(ContentsTarball), @@ -140,8 +141,9 @@ create_docs(Files, Config) -> docs_tarball_max_size := TarballMaxSize, docs_tarball_max_uncompressed_size := TarballMaxUncompressedSize } = Config, + FilesRoot = maps:get(tarball_files_root, Config, undefined), - case validate_create_files(Files) of + case validate_create_files(Files, FilesRoot) of ok -> UncompressedTarball = create_memory_tarball(Files), @@ -900,23 +902,27 @@ guess_build_tools(Metadata) -> %%==================================================================== %% @private -validate_create_files(Files) when is_list(Files) -> - validate_create_files(Files, ok). +validate_create_files(Files, FilesRoot) when is_list(Files) -> + validate_create_files(Files, FilesRoot, ok). -validate_create_files(_Files, {error, _} = Error) -> +validate_create_files(_Files, _FilesRoot, {error, _} = Error) -> Error; -validate_create_files([], ok) -> +validate_create_files([], _FilesRoot, ok) -> ok; -validate_create_files([File | Rest], ok) -> - validate_create_files(Rest, validate_create_file(File)). +validate_create_files([File | Rest], FilesRoot, ok) -> + validate_create_files(Rest, FilesRoot, validate_create_file(File, FilesRoot)). -validate_create_file({Filename, Contents}) when is_list(Filename), is_binary(Contents) -> +validate_create_file({Filename, Contents}, _FilesRoot) when + is_list(Filename), is_binary(Contents) +-> validate_archive_path(Filename); -validate_create_file(Filename) when is_list(Filename) -> - validate_create_file({Filename, Filename}); -validate_create_file({Filename, AbsFilename}) when is_list(Filename), is_list(AbsFilename) -> +validate_create_file(Filename, FilesRoot) when is_list(Filename) -> + validate_create_file({Filename, Filename}, FilesRoot); +validate_create_file({Filename, AbsFilename}, FilesRoot) when + is_list(Filename), is_list(AbsFilename) +-> case validate_archive_path(Filename) of - ok -> validate_source_file(Filename, AbsFilename); + ok -> validate_source_file(Filename, AbsFilename, FilesRoot); {error, _} = Error -> Error end. @@ -926,13 +932,13 @@ validate_archive_path(Filename) -> true -> ok end. -validate_source_file(ArchiveName, SourcePath) -> +validate_source_file(ArchiveName, SourcePath, FilesRoot) -> case file:read_link_info(SourcePath, []) of {ok, #file_info{type = Type}} when Type =:= regular; Type =:= directory -> - ok; + validate_source_root(ArchiveName, SourcePath, FilesRoot); {ok, #file_info{type = symlink}} -> {ok, LinkTarget} = file:read_link(SourcePath), - ResolvedTarget = filename:join(filename:dirname(ArchiveName), LinkTarget), + ResolvedTarget = archive_join(archive_dirname(ArchiveName), LinkTarget), case safe_relative_archive_path(ResolvedTarget) of false -> {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}}; true -> ok @@ -943,10 +949,18 @@ validate_source_file(ArchiveName, SourcePath) -> ok end. +validate_source_root(_ArchiveName, _SourcePath, undefined) -> + ok; +validate_source_root(ArchiveName, SourcePath, FilesRoot) -> + case filelib:safe_relative_path(SourcePath, filename:absname(FilesRoot)) of + unsafe -> {error, {tarball, {unsafe_path, ArchiveName}}}; + _ -> ok + end. + safe_relative_archive_path(Path) -> - case filename:pathtype(Path) of - relative -> safe_relative_archive_path(filename:split(Path), []); - _ -> false + case archive_path_absolute(Path) orelse archive_path_drive(Path) of + true -> false; + false -> safe_relative_archive_path(archive_path_split(Path), []) end. safe_relative_archive_path([], _Acc) -> @@ -960,6 +974,47 @@ safe_relative_archive_path([".." | Rest], [_ | Acc]) -> safe_relative_archive_path([_Part | Rest], Acc) -> safe_relative_archive_path(Rest, [ok | Acc]). +archive_path_absolute([$/ | _Rest]) -> + true; +archive_path_absolute([$\\ | _Rest]) -> + true; +archive_path_absolute(_Path) -> + false. + +archive_path_drive([Drive, $: | _Rest]) when + Drive >= $a, Drive =< $z; + Drive >= $A, Drive =< $Z +-> + true; +archive_path_drive(_Path) -> + false. + +archive_path_split(Path) -> + string:tokens(Path, "/\\"). + +archive_dirname(Path) -> + case archive_path_split(Path) of + [] -> "."; + [_Name] -> "."; + Parts -> string:join(lists:droplast(Parts), "/") + end. + +archive_join(_Dir, Path) when Path =:= [] -> + Path; +archive_join(_Dir, Path = [$/ | _Rest]) -> + Path; +archive_join(_Dir, Path = [$\\ | _Rest]) -> + Path; +archive_join(_Dir, Path = [Drive, $: | _Rest]) when + Drive >= $a, Drive =< $z; + Drive >= $A, Drive =< $Z +-> + Path; +archive_join(".", Path) -> + Path; +archive_join(Dir, Path) -> + Dir ++ "/" ++ Path. + %% @private unpack_tarball(Source, memory, MaxSize) -> case hex_erl_tar:extract(Source, [memory, compressed, {max_size, MaxSize}]) of diff --git a/test/hex_tarball_SUITE.erl b/test/hex_tarball_SUITE.erl index f4b02b03..2e91dbc7 100644 --- a/test/hex_tarball_SUITE.erl +++ b/test/hex_tarball_SUITE.erl @@ -282,10 +282,18 @@ unsafe_paths_to_create_test(Config) -> hex_tarball:create(Metadata, [{"../README.md", <<"README">>}]), {error, {tarball, {unsafe_path, "/README.md"}}} = hex_tarball:create(Metadata, [{"/README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "C:\\README.md"}}} = + hex_tarball:create(Metadata, [{"C:\\README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "..\\README.md"}}} = + hex_tarball:create(Metadata, [{"..\\README.md", <<"README">>}]), {error, {tarball, {unsafe_path, "../README.md"}}} = hex_tarball:create_docs([{"../README.md", <<"README">>}]), {error, {tarball, {unsafe_path, "/README.md"}}} = hex_tarball:create_docs([{"/README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "C:\\README.md"}}} = + hex_tarball:create_docs([{"C:\\README.md", <<"README">>}]), + {error, {tarball, {unsafe_path, "..\\README.md"}}} = + hex_tarball:create_docs([{"..\\README.md", <<"README">>}]), UnsafeLink = filename:join(BaseDir, "unsafe_link"), ok = file:make_symlink("../../README.md", UnsafeLink), @@ -294,6 +302,25 @@ unsafe_paths_to_create_test(Config) -> {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = hex_tarball:create_docs([{"README.md", UnsafeLink}]), + RootDir = filename:join(BaseDir, "source_root"), + OutsideDir = filename:join(BaseDir, "outside"), + ok = file:make_dir(RootDir), + ok = file:make_dir(OutsideDir), + ok = file:write_file(filename:join(OutsideDir, "secret.txt"), <<"secret">>), + ok = file:make_symlink("../outside", filename:join(RootDir, "link")), + CreateConfig = maps:put(tarball_files_root, RootDir, hex_core:default_config()), + {error, {tarball, {unsafe_path, "link/secret.txt"}}} = + hex_tarball:create( + Metadata, + [{"link/secret.txt", filename:join([RootDir, "link", "secret.txt"])}], + CreateConfig + ), + {error, {tarball, {unsafe_path, "link/secret.txt"}}} = + hex_tarball:create_docs( + [{"link/secret.txt", filename:join([RootDir, "link", "secret.txt"])}], + CreateConfig + ), + ok. unsupported_file_types_to_create_test(Config) -> From 4a6a1e969adda7704c92dc3ede6747d9c9a80c71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Eric=20Meadows-J=C3=B6nsson?= Date: Wed, 13 May 2026 01:33:48 +0200 Subject: [PATCH 5/5] Require root-relative tarball source paths --- src/hex_core.erl | 6 ++- src/hex_tarball.erl | 70 ++++++++++++++++++++++---------- test/hex_tarball_SUITE.erl | 83 ++++++++++++++++++++++++++++---------- 3 files changed, 114 insertions(+), 45 deletions(-) diff --git a/src/hex_core.erl b/src/hex_core.erl index 83c3b3ab..ad24ebd6 100644 --- a/src/hex_core.erl +++ b/src/hex_core.erl @@ -66,8 +66,10 @@ %% * `tarball_max_uncompressed_size' - Maximum size of uncompressed package tarball, defaults to %% `134_217_728' (128 MiB). Set to `infinity' to not enforce the limit. %% -%% * `tarball_files_root' - Root directory source files must resolve inside when creating tarballs. -%% Set to `undefined' to skip source root validation (default: `undefined'). +%% * `tarball_files_root' - Root directory for source files when creating tarballs. +%% Required for filesystem source paths, which must be relative and must resolve inside +%% this root after following symlinks. Set to `undefined' when all tarball contents are +%% provided as binaries and no filesystem source paths are used (default: `undefined'). %% %% * `docs_tarball_max_size' - Maximum size of docs tarball, defaults to %% `16_777_216' (16 MiB). Set to `infinity' to not enforce the limit. diff --git a/src/hex_tarball.erl b/src/hex_tarball.erl index 3d54a96e..d5254b33 100644 --- a/src/hex_tarball.erl +++ b/src/hex_tarball.erl @@ -76,8 +76,8 @@ create(Metadata, Files, Config) -> {error, {tarball, {file_too_big, "metadata.config"}}}; true -> case validate_create_files(Files, FilesRoot) of - ok -> - ContentsTarball = create_memory_tarball(Files), + {ok, ValidatedFiles} -> + ContentsTarball = create_memory_tarball(ValidatedFiles), ContentsTarballCompressed = gzip(ContentsTarball), InnerChecksum = inner_checksum( ?VERSION, MetadataBinary, ContentsTarballCompressed @@ -144,8 +144,8 @@ create_docs(Files, Config) -> FilesRoot = maps:get(tarball_files_root, Config, undefined), case validate_create_files(Files, FilesRoot) of - ok -> - UncompressedTarball = create_memory_tarball(Files), + {ok, ValidatedFiles} -> + UncompressedTarball = create_memory_tarball(ValidatedFiles), case valid_size(UncompressedTarball, TarballMaxUncompressedSize) of true -> @@ -354,6 +354,8 @@ format_error({tarball, {too_big_compressed, Size}}) -> io_lib:format("package exceeds max compressed size ~w ~s", [format_byte_size(Size), "MB"]); format_error({tarball, {missing_files, Files}}) -> io_lib:format("missing files: ~p", [Files]); +format_error({tarball, missing_files_root}) -> + "tarball files root is required when creating tarballs from filesystem paths"; format_error({tarball, {bad_version, Vsn}}) -> io_lib:format("unsupported version: ~p", [Vsn]); format_error({tarball, invalid_checksum}) -> @@ -903,19 +905,23 @@ guess_build_tools(Metadata) -> %% @private validate_create_files(Files, FilesRoot) when is_list(Files) -> - validate_create_files(Files, FilesRoot, ok). + validate_create_files(Files, FilesRoot, []). -validate_create_files(_Files, _FilesRoot, {error, _} = Error) -> - Error; -validate_create_files([], _FilesRoot, ok) -> - ok; -validate_create_files([File | Rest], FilesRoot, ok) -> - validate_create_files(Rest, FilesRoot, validate_create_file(File, FilesRoot)). +validate_create_files([], _FilesRoot, Acc) -> + {ok, lists:reverse(Acc)}; +validate_create_files([File | Rest], FilesRoot, Acc) -> + case validate_create_file(File, FilesRoot) of + {ok, ValidatedFile} -> validate_create_files(Rest, FilesRoot, [ValidatedFile | Acc]); + {error, _} = Error -> Error + end. validate_create_file({Filename, Contents}, _FilesRoot) when is_list(Filename), is_binary(Contents) -> - validate_archive_path(Filename); + case validate_archive_path(Filename) of + ok -> {ok, {Filename, Contents}}; + {error, _} = Error -> Error + end; validate_create_file(Filename, FilesRoot) when is_list(Filename) -> validate_create_file({Filename, Filename}, FilesRoot); validate_create_file({Filename, AbsFilename}, FilesRoot) when @@ -933,26 +939,48 @@ validate_archive_path(Filename) -> end. validate_source_file(ArchiveName, SourcePath, FilesRoot) -> - case file:read_link_info(SourcePath, []) of + case validate_source_path(SourcePath) of + ok -> validate_source_file_root(ArchiveName, SourcePath, FilesRoot); + {error, _} = Error -> Error + end. + +validate_source_path(SourcePath) -> + case safe_relative_archive_path(SourcePath) of + false -> {error, {tarball, {unsafe_path, SourcePath}}}; + true -> ok + end. + +validate_source_file_root(_ArchiveName, _SourcePath, undefined) -> + {error, {tarball, missing_files_root}}; +validate_source_file_root(ArchiveName, SourcePath, FilesRoot) -> + Root = filename:absname(FilesRoot), + DiskPath = filename:join(Root, SourcePath), + case file:read_link_info(DiskPath, []) of {ok, #file_info{type = Type}} when Type =:= regular; Type =:= directory -> - validate_source_root(ArchiveName, SourcePath, FilesRoot); + case validate_source_root(ArchiveName, SourcePath, Root) of + ok -> {ok, {ArchiveName, DiskPath}}; + {error, _} = Error -> Error + end; {ok, #file_info{type = symlink}} -> - {ok, LinkTarget} = file:read_link(SourcePath), + {ok, LinkTarget} = file:read_link(DiskPath), ResolvedTarget = archive_join(archive_dirname(ArchiveName), LinkTarget), case safe_relative_archive_path(ResolvedTarget) of - false -> {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}}; - true -> ok + false -> + {error, {tarball, {unsafe_symlink, ArchiveName, LinkTarget}}}; + true -> + case validate_source_root(ArchiveName, SourcePath, Root) of + ok -> {ok, {ArchiveName, DiskPath}}; + {error, _} = Error -> Error + end end; {ok, #file_info{type = Type}} -> {error, {tarball, {unsupported_file_type, ArchiveName, Type}}}; _ -> - ok + {ok, {ArchiveName, DiskPath}} end. -validate_source_root(_ArchiveName, _SourcePath, undefined) -> - ok; validate_source_root(ArchiveName, SourcePath, FilesRoot) -> - case filelib:safe_relative_path(SourcePath, filename:absname(FilesRoot)) of + case filelib:safe_relative_path(SourcePath, FilesRoot) of unsafe -> {error, {tarball, {unsafe_path, ArchiveName}}}; _ -> ok end. diff --git a/test/hex_tarball_SUITE.erl b/test/hex_tarball_SUITE.erl index 2e91dbc7..5775e66c 100644 --- a/test/hex_tarball_SUITE.erl +++ b/test/hex_tarball_SUITE.erl @@ -106,12 +106,13 @@ disk_test(Config) -> ok = file:change_mode(Foo, 8#100644), ok = file:write_file(filename:join(SrcDir, "not_whitelisted.erl"), <<"">>), - Files = [{"empty", EmptyDir}, {"src", SrcDir}, {"src/foo.erl", Foo}], + Files = [{"empty", "empty"}, {"src", "src"}, {"src/foo.erl", filename:join("src", "foo.erl")}], Metadata = #{ <<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>, <<"build_tool">> => <<"rebar3">> }, + CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), {ok, #{tarball := Tarball, inner_checksum := InnerChecksum, outer_checksum := OuterChecksum}} = hex_tarball:create( - Metadata, Files + Metadata, Files, CreateConfig ), ?assertEqual( <<"3DED88F7BAC738294907ACEED89FE63C9914713F988E608F2B17D7AE6EAC8446">>, @@ -144,13 +145,14 @@ timestamps_and_permissions_test(Config) -> ok = file:make_dir(EmptyDir), ok = file:change_mode(EmptyDir, 8#100755), Files = [ - {"empty", EmptyDir}, + {"empty", "timestamps_empty"}, {"foo.erl", <<"">>}, - {"foo.sh", Foo} + {"foo.sh", "foo.sh"} ], + CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), {ok, #{tarball := Tarball, inner_checksum := InnerChecksum, outer_checksum := OuterChecksum}} = hex_tarball:create( - Metadata, Files + Metadata, Files, CreateConfig ), %% inside tarball @@ -191,12 +193,13 @@ symlinks_test(Config) -> ok = file:make_symlink("foo.sh", BarSh), Files = [ - {"dir/foo.sh", FooSh}, - {"dir/bar.sh", BarSh} + {"dir/foo.sh", filename:join("dir", "foo.sh")}, + {"dir/bar.sh", filename:join("dir", "bar.sh")} ], + CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), {ok, #{tarball := Tarball, inner_checksum := InnerChecksum, outer_checksum := OuterChecksum}} = hex_tarball:create( - Metadata, Files + Metadata, Files, CreateConfig ), UnpackDir = filename:join(BaseDir, "symlinks"), {ok, #{inner_checksum := InnerChecksum, outer_checksum := OuterChecksum}} = hex_tarball:unpack( @@ -229,11 +232,12 @@ symlinks_parent_dir_test(Config) -> ok = file:make_symlink("../foo2.sh", LinkSh), Files = [ - {"foo.sh", FooSh}, - {"dir/link.sh", LinkSh} + {"foo.sh", "foo2.sh"}, + {"dir/link.sh", filename:join("dir2", "link.sh")} ], + CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), - {ok, #{tarball := Tarball}} = hex_tarball:create(Metadata, Files), + {ok, #{tarball := Tarball}} = hex_tarball:create(Metadata, Files, CreateConfig), {ok, _} = hex_tarball:unpack(Tarball, memory), UnpackDir = filename:join(BaseDir, "symlinks_parent_dir"), @@ -251,11 +255,11 @@ symlinks_parent_dir_test(Config) -> ok = file:make_symlink("../../foo3.sh", LinkSh2), Files2 = [ - {"foo.sh", FooSh2}, - {"a/b/link.sh", LinkSh2} + {"foo.sh", "foo3.sh"}, + {"a/b/link.sh", filename:join(["a2", "b2", "link.sh"])} ], - {ok, #{tarball := Tarball2}} = hex_tarball:create(Metadata, Files2), + {ok, #{tarball := Tarball2}} = hex_tarball:create(Metadata, Files2, CreateConfig), UnpackDir2 = filename:join(BaseDir, "symlinks_parent_dir2"), {ok, _} = hex_tarball:unpack(Tarball2, UnpackDir2), {ok, #file_info{type = symlink}} = @@ -268,9 +272,9 @@ symlinks_parent_dir_test(Config) -> ok = file:make_dir(UnsafeDir), ok = file:make_symlink("../../escape", UnsafeLink), - UnsafeFiles = [{"dir/link.sh", UnsafeLink}], + UnsafeFiles = [{"dir/link.sh", filename:join("unsafe_dir", "link.sh")}], {error, {tarball, {unsafe_symlink, "dir/link.sh", "../../escape"}}} = - hex_tarball:create(Metadata, UnsafeFiles), + hex_tarball:create(Metadata, UnsafeFiles, CreateConfig), ok. @@ -297,27 +301,61 @@ unsafe_paths_to_create_test(Config) -> UnsafeLink = filename:join(BaseDir, "unsafe_link"), ok = file:make_symlink("../../README.md", UnsafeLink), + BaseConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = - hex_tarball:create(Metadata, [{"README.md", UnsafeLink}]), + hex_tarball:create(Metadata, [{"README.md", "unsafe_link"}], BaseConfig), {error, {tarball, {unsafe_symlink, "README.md", "../../README.md"}}} = - hex_tarball:create_docs([{"README.md", UnsafeLink}]), + hex_tarball:create_docs([{"README.md", "unsafe_link"}], BaseConfig), RootDir = filename:join(BaseDir, "source_root"), OutsideDir = filename:join(BaseDir, "outside"), ok = file:make_dir(RootDir), ok = file:make_dir(OutsideDir), + ok = file:write_file(filename:join(RootDir, "README.md"), <<"README">>), ok = file:write_file(filename:join(OutsideDir, "secret.txt"), <<"secret">>), ok = file:make_symlink("../outside", filename:join(RootDir, "link")), CreateConfig = maps:put(tarball_files_root, RootDir, hex_core:default_config()), + RootReadme = filename:join(RootDir, "README.md"), + {error, {tarball, missing_files_root}} = + hex_tarball:create(Metadata, [{"README.md", "README.md"}]), + {error, {tarball, missing_files_root}} = + hex_tarball:create_docs([{"README.md", "README.md"}]), + {error, {tarball, {unsafe_path, RootReadme}}} = + hex_tarball:create( + Metadata, + [{"README.md", RootReadme}], + CreateConfig + ), + {error, {tarball, {unsafe_path, RootReadme}}} = + hex_tarball:create_docs( + [{"README.md", RootReadme}], + CreateConfig + ), + {ok, _} = + hex_tarball:create(Metadata, [{"README.md", "README.md"}], CreateConfig), + {ok, _} = + hex_tarball:create_docs([{"README.md", "README.md"}], CreateConfig), + ok = file:make_symlink("../outside/secret.txt", filename:join(RootDir, "mismatch_link")), + {error, {tarball, {unsafe_path, "nested/mismatch_link"}}} = + hex_tarball:create( + Metadata, + [{"nested/mismatch_link", "mismatch_link"}], + CreateConfig + ), + {error, {tarball, {unsafe_path, "nested/mismatch_link"}}} = + hex_tarball:create_docs( + [{"nested/mismatch_link", "mismatch_link"}], + CreateConfig + ), {error, {tarball, {unsafe_path, "link/secret.txt"}}} = hex_tarball:create( Metadata, - [{"link/secret.txt", filename:join([RootDir, "link", "secret.txt"])}], + [{"link/secret.txt", "link/secret.txt"}], CreateConfig ), {error, {tarball, {unsafe_path, "link/secret.txt"}}} = hex_tarball:create_docs( - [{"link/secret.txt", filename:join([RootDir, "link", "secret.txt"])}], + [{"link/secret.txt", "link/secret.txt"}], CreateConfig ), @@ -331,13 +369,14 @@ unsupported_file_types_to_create_test(Config) -> BaseDir = ?config(priv_dir, Config), Metadata = #{<<"name">> => <<"foo">>, <<"version">> => <<"1.0.0">>}, Fifo = filename:join(BaseDir, "fifo"), + CreateConfig = maps:put(tarball_files_root, BaseDir, hex_core:default_config()), _ = file:delete(Fifo), [] = os:cmd(Mkfifo ++ " " ++ shell_quote(Fifo)), {error, {tarball, {unsupported_file_type, "fifo", other}}} = - hex_tarball:create(Metadata, [{"fifo", Fifo}]), + hex_tarball:create(Metadata, [{"fifo", "fifo"}], CreateConfig), {error, {tarball, {unsupported_file_type, "fifo", other}}} = - hex_tarball:create_docs([{"fifo", Fifo}]), + hex_tarball:create_docs([{"fifo", "fifo"}], CreateConfig), ok end.