Skip to content

Commit 28cfc17

Browse files
ericmjericmj
authored
Add file-based unpack and size validations for tarball files (#166)
Add support for unpacking tarballs from file paths via {file, Path} input to avoid loading the entire tarball into memory. When used with memory output, the inner tarball contents are not decompressed, reducing peak memory from ~160MB to <1MB per publish request. Also add size validations for outer tarball entries (VERSION, CHECKSUM, metadata.config) during both creation and extraction to reject malformed tarballs early.
1 parent 961730b commit 28cfc17

3 files changed

Lines changed: 380 additions & 49 deletions

File tree

‎.github/workflows/main.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@ jobs:
5353

5454
- run: rebar3 ct
5555
- run: rebar3 as test proper
56-
- run: rm src/safe_erl_term.erl && rebar3 fmt --check
56+
- run: rebar3 fmt --check
5757
if: ${{ matrix.pair.lint }}

‎src/hex_tarball.erl‎

Lines changed: 257 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,10 @@
1313
-export([do_decode_metadata/1, gzip/1, normalize_requirements/1]).
1414
-endif.
1515
-define(VERSION, <<"3">>).
16+
-define(HASH_CHUNK_SIZE, 65536).
17+
-define(MAX_VERSION_SIZE, 32).
18+
-define(MAX_CHECKSUM_SIZE, 128).
19+
-define(MAX_METADATA_SIZE, 128 * 1024).
1620
-define(BUILD_TOOL_FILES, [
1721
{<<"mix.exs">>, <<"mix">>},
1822
{<<"rebar.config">>, <<"rebar3">>},
@@ -64,37 +68,41 @@ create(Metadata, Files, Config) ->
6468
} = Config,
6569

6670
MetadataBinary = encode_metadata(Metadata),
67-
ContentsTarball = create_memory_tarball(Files),
68-
ContentsTarballCompressed = gzip(ContentsTarball),
69-
InnerChecksum = inner_checksum(?VERSION, MetadataBinary, ContentsTarballCompressed),
70-
InnerChecksumBase16 = encode_base16(InnerChecksum),
71-
TarballMaxSize = maps:get(tarball_max_size, Config),
72-
TarballMaxUncompressedSize = maps:get(tarball_max_uncompressed_size, Config),
73-
74-
OuterFiles = [
75-
{"VERSION", ?VERSION},
76-
{"CHECKSUM", InnerChecksumBase16},
77-
{"metadata.config", MetadataBinary},
78-
{"contents.tar.gz", ContentsTarballCompressed}
79-
],
8071

81-
case valid_size(ContentsTarball, TarballMaxUncompressedSize) of
72+
case valid_size(MetadataBinary, ?MAX_METADATA_SIZE) of
73+
false ->
74+
{error, {tarball, {file_too_big, "metadata.config"}}};
8275
true ->
83-
Tarball = create_memory_tarball(OuterFiles),
84-
OuterChecksum = checksum(Tarball),
76+
ContentsTarball = create_memory_tarball(Files),
77+
ContentsTarballCompressed = gzip(ContentsTarball),
78+
InnerChecksum = inner_checksum(?VERSION, MetadataBinary, ContentsTarballCompressed),
79+
InnerChecksumBase16 = encode_base16(InnerChecksum),
80+
81+
OuterFiles = [
82+
{"VERSION", ?VERSION},
83+
{"CHECKSUM", InnerChecksumBase16},
84+
{"metadata.config", MetadataBinary},
85+
{"contents.tar.gz", ContentsTarballCompressed}
86+
],
8587

86-
case valid_size(Tarball, TarballMaxSize) of
88+
case valid_size(ContentsTarball, TarballMaxUncompressedSize) of
8789
true ->
88-
{ok, #{
89-
tarball => Tarball,
90-
outer_checksum => OuterChecksum,
91-
inner_checksum => InnerChecksum
92-
}};
90+
Tarball = create_memory_tarball(OuterFiles),
91+
OuterChecksum = checksum(Tarball),
92+
93+
case valid_size(Tarball, TarballMaxSize) of
94+
true ->
95+
{ok, #{
96+
tarball => Tarball,
97+
outer_checksum => OuterChecksum,
98+
inner_checksum => InnerChecksum
99+
}};
100+
false ->
101+
{error, {tarball, {too_big_compressed, TarballMaxSize}}}
102+
end;
93103
false ->
94-
{error, {tarball, {too_big_compressed, TarballMaxSize}}}
95-
end;
96-
false ->
97-
{error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}}
104+
{error, {tarball, {too_big_uncompressed, TarballMaxUncompressedSize}}}
105+
end
98106
end.
99107

100108
-spec create(metadata(), files()) ->
@@ -179,6 +187,30 @@ create_docs(Files) ->
179187
metadata => metadata()
180188
}}
181189
| {error, term()}.
190+
unpack({file, Path}, Output, Config) ->
191+
case valid_file_size(Path, maps:get(tarball_max_size, Config)) of
192+
true ->
193+
TmpDir = tmp_path(),
194+
ok = file:make_dir(TmpDir),
195+
try
196+
case hex_erl_tar:extract(Path, [{cwd, TmpDir}]) of
197+
ok ->
198+
OuterChecksum = file_checksum(Path),
199+
case read_outer_files_from_dir(TmpDir) of
200+
{ok, Files} ->
201+
do_unpack(Files, OuterChecksum, Output);
202+
{error, _} = Error ->
203+
Error
204+
end;
205+
{error, Reason} ->
206+
{error, {tarball, Reason}}
207+
end
208+
after
209+
remove_dir(TmpDir)
210+
end;
211+
false ->
212+
{error, {tarball, too_big}}
213+
end;
182214
unpack(Tarball, Output, Config) ->
183215
case valid_size(Tarball, maps:get(tarball_max_size, Config)) of
184216
true ->
@@ -187,7 +219,9 @@ unpack(Tarball, Output, Config) ->
187219
{error, {tarball, empty}};
188220
{ok, FileList} ->
189221
OuterChecksum = crypto:hash(sha256, Tarball),
190-
do_unpack(maps:from_list(FileList), OuterChecksum, Output);
222+
do_unpack(
223+
validate_outer_file_sizes(maps:from_list(FileList)), OuterChecksum, Output
224+
);
191225
{error, Reason} ->
192226
{error, {tarball, Reason}}
193227
end;
@@ -256,6 +290,8 @@ format_checksum(Checksum) ->
256290
%% @doc
257291
%% Converts an error reason term to a human-readable error message string.
258292
-spec format_error(term()) -> string().
293+
format_error({tarball, {file_too_big, Name}}) ->
294+
io_lib:format("file too big: ~s", [Name]);
259295
format_error({tarball, empty}) ->
260296
"empty tarball";
261297
format_error({tarball, {too_big_uncompressed, Size}}) ->
@@ -294,6 +330,12 @@ format_byte_size(Size) ->
294330
%%====================================================================
295331

296332
%% @private
333+
inner_checksum(Version, MetadataBinary, {path, ContentsPath}) ->
334+
HashState0 = crypto:hash_init(sha256),
335+
HashState1 = crypto:hash_update(HashState0, Version),
336+
HashState2 = crypto:hash_update(HashState1, MetadataBinary),
337+
HashState3 = stream_file_hash(HashState2, ContentsPath),
338+
crypto:hash_final(HashState3);
297339
inner_checksum(Version, MetadataBinary, ContentsBinary) ->
298340
Blob = <<Version/binary, MetadataBinary/binary, ContentsBinary/binary>>,
299341
crypto:hash(sha256, Blob).
@@ -314,6 +356,8 @@ encode_metadata(Meta) ->
314356
iolist_to_binary(Data).
315357

316358
%% @private
359+
do_unpack({error, _} = Error, _OuterChecksum, _Output) ->
360+
Error;
317361
do_unpack(Files, OuterChecksum, Output) ->
318362
State = #{
319363
inner_checksum => undefined,
@@ -340,30 +384,54 @@ finish_unpack(#{
340384
output := Output
341385
}) ->
342386
_ = maps:get("VERSION", Files),
343-
ContentsBinary = maps:get("contents.tar.gz", Files),
344-
345-
case Output of
346-
memory -> ok;
347-
_ -> filelib:ensure_dir(filename:join(Output, "*"))
348-
end,
387+
Contents = maps:get("contents.tar.gz", Files),
349388

350-
case unpack_tarball(ContentsBinary, Output) of
351-
ok ->
352-
copy_metadata_config(Output, maps:get("metadata.config", Files)),
389+
case {Contents, Output} of
390+
{{path, _}, memory} ->
353391
{ok, #{
354392
inner_checksum => InnerChecksum,
355393
outer_checksum => OuterChecksum,
356394
metadata => Metadata
357395
}};
358-
{ok, Contents} ->
359-
{ok, #{
360-
inner_checksum => InnerChecksum,
361-
outer_checksum => OuterChecksum,
362-
metadata => Metadata,
363-
contents => Contents
364-
}};
365-
{error, Reason} ->
366-
{error, {inner_tarball, Reason}}
396+
{{path, ContentsPath}, _} ->
397+
filelib:ensure_dir(filename:join(Output, "*")),
398+
{ok, ContentsBinary} = file:read_file(ContentsPath),
399+
case unpack_tarball(ContentsBinary, Output) of
400+
ok ->
401+
copy_metadata_config(Output, maps:get("metadata.config", Files)),
402+
{ok, #{
403+
inner_checksum => InnerChecksum,
404+
outer_checksum => OuterChecksum,
405+
metadata => Metadata
406+
}};
407+
{error, Reason} ->
408+
{error, {inner_tarball, Reason}}
409+
end;
410+
{ContentsBinary, memory} ->
411+
case unpack_tarball(ContentsBinary, Output) of
412+
{ok, UnpackedContents} ->
413+
{ok, #{
414+
inner_checksum => InnerChecksum,
415+
outer_checksum => OuterChecksum,
416+
metadata => Metadata,
417+
contents => UnpackedContents
418+
}};
419+
{error, Reason} ->
420+
{error, {inner_tarball, Reason}}
421+
end;
422+
{ContentsBinary, _} ->
423+
filelib:ensure_dir(filename:join(Output, "*")),
424+
case unpack_tarball(ContentsBinary, Output) of
425+
ok ->
426+
copy_metadata_config(Output, maps:get("metadata.config", Files)),
427+
{ok, #{
428+
inner_checksum => InnerChecksum,
429+
outer_checksum => OuterChecksum,
430+
metadata => Metadata
431+
}};
432+
{error, Reason} ->
433+
{error, {inner_tarball, Reason}}
434+
end
367435
end.
368436

369437
%% @private
@@ -401,8 +469,8 @@ check_inner_checksum(#{files := Files} = State) ->
401469

402470
Version = maps:get("VERSION", Files),
403471
MetadataBinary = maps:get("metadata.config", Files),
404-
ContentsBinary = maps:get("contents.tar.gz", Files),
405-
ActualChecksum = inner_checksum(Version, MetadataBinary, ContentsBinary),
472+
Contents = maps:get("contents.tar.gz", Files),
473+
ActualChecksum = inner_checksum(Version, MetadataBinary, Contents),
406474

407475
if
408476
byte_size(ExpectedChecksum) /= 32 ->
@@ -622,6 +690,148 @@ valid_size(Binary, infinity) when is_binary(Binary) ->
622690
valid_size(Binary, Limit) when is_binary(Binary) and is_integer(Limit) ->
623691
byte_size(Binary) =< Limit.
624692

693+
%% @private
694+
valid_file_size(_Path, infinity) ->
695+
true;
696+
valid_file_size(Path, Limit) when is_integer(Limit) ->
697+
case file:read_file_info(Path) of
698+
{ok, #file_info{size = Size}} -> Size =< Limit;
699+
{error, _} -> false
700+
end.
701+
702+
%% @private
703+
file_checksum(Path) ->
704+
{ok, Fd} = file:open(Path, [read, raw, binary]),
705+
try
706+
file_checksum_loop(Fd, crypto:hash_init(sha256))
707+
after
708+
file:close(Fd)
709+
end.
710+
711+
%% @private
712+
file_checksum_loop(Fd, HashState) ->
713+
case file:read(Fd, ?HASH_CHUNK_SIZE) of
714+
{ok, Data} -> file_checksum_loop(Fd, crypto:hash_update(HashState, Data));
715+
eof -> crypto:hash_final(HashState)
716+
end.
717+
718+
%% @private
719+
stream_file_hash(HashState, Path) ->
720+
{ok, Fd} = file:open(Path, [read, raw, binary]),
721+
try
722+
stream_file_hash_loop(Fd, HashState)
723+
after
724+
file:close(Fd)
725+
end.
726+
727+
%% @private
728+
stream_file_hash_loop(Fd, HashState) ->
729+
case file:read(Fd, ?HASH_CHUNK_SIZE) of
730+
{ok, Data} -> stream_file_hash_loop(Fd, crypto:hash_update(HashState, Data));
731+
eof -> HashState
732+
end.
733+
734+
%% @private
735+
read_outer_files_from_dir(Dir) ->
736+
VersionPath = filename:join(Dir, "VERSION"),
737+
ChecksumPath = filename:join(Dir, "CHECKSUM"),
738+
MetadataPath = filename:join(Dir, "metadata.config"),
739+
ContentsPath = filename:join(Dir, "contents.tar.gz"),
740+
741+
case
742+
{
743+
filelib:is_regular(VersionPath),
744+
filelib:is_regular(ChecksumPath),
745+
filelib:is_regular(MetadataPath),
746+
filelib:is_regular(ContentsPath)
747+
}
748+
of
749+
{true, true, true, true} ->
750+
case check_outer_file_sizes(VersionPath, ChecksumPath, MetadataPath) of
751+
ok ->
752+
{ok, Version} = file:read_file(VersionPath),
753+
{ok, Checksum} = file:read_file(ChecksumPath),
754+
{ok, MetadataConfig} = file:read_file(MetadataPath),
755+
{ok, #{
756+
"VERSION" => Version,
757+
"CHECKSUM" => Checksum,
758+
"metadata.config" => MetadataConfig,
759+
"contents.tar.gz" => {path, ContentsPath}
760+
}};
761+
{error, _} = Error ->
762+
Error
763+
end;
764+
_ ->
765+
Missing = lists:filtermap(
766+
fun({Path, Name}) ->
767+
case filelib:is_regular(Path) of
768+
true -> false;
769+
false -> {true, Name}
770+
end
771+
end,
772+
[
773+
{VersionPath, "VERSION"},
774+
{ChecksumPath, "CHECKSUM"},
775+
{MetadataPath, "metadata.config"},
776+
{ContentsPath, "contents.tar.gz"}
777+
]
778+
),
779+
{error, {tarball, {missing_files, Missing}}}
780+
end.
781+
782+
%% @private
783+
check_outer_file_sizes(VersionPath, ChecksumPath, MetadataPath) ->
784+
case valid_file_size(VersionPath, ?MAX_VERSION_SIZE) of
785+
false ->
786+
{error, {tarball, {file_too_big, "VERSION"}}};
787+
true ->
788+
case valid_file_size(ChecksumPath, ?MAX_CHECKSUM_SIZE) of
789+
false ->
790+
{error, {tarball, {file_too_big, "CHECKSUM"}}};
791+
true ->
792+
case valid_file_size(MetadataPath, ?MAX_METADATA_SIZE) of
793+
false -> {error, {tarball, {file_too_big, "metadata.config"}}};
794+
true -> ok
795+
end
796+
end
797+
end.
798+
799+
%% @private
800+
validate_outer_file_sizes(Files) ->
801+
case byte_size(maps:get("VERSION", Files, <<>>)) > ?MAX_VERSION_SIZE of
802+
true ->
803+
{error, {tarball, {file_too_big, "VERSION"}}};
804+
false ->
805+
case byte_size(maps:get("CHECKSUM", Files, <<>>)) > ?MAX_CHECKSUM_SIZE of
806+
true ->
807+
{error, {tarball, {file_too_big, "CHECKSUM"}}};
808+
false ->
809+
case byte_size(maps:get("metadata.config", Files, <<>>)) > ?MAX_METADATA_SIZE of
810+
true -> {error, {tarball, {file_too_big, "metadata.config"}}};
811+
false -> Files
812+
end
813+
end
814+
end.
815+
816+
%% @private
817+
remove_dir(Dir) ->
818+
case file:list_dir(Dir) of
819+
{ok, Entries} ->
820+
lists:foreach(
821+
fun(Entry) ->
822+
Path = filename:join(Dir, Entry),
823+
case filelib:is_dir(Path) of
824+
true -> remove_dir(Path);
825+
false -> file:delete(Path)
826+
end
827+
end,
828+
Entries
829+
),
830+
file:del_dir(Dir);
831+
{error, _} ->
832+
ok
833+
end.
834+
625835
%% @private
626836
binarify(Binary) when is_binary(Binary) -> Binary;
627837
binarify(Number) when is_number(Number) -> Number;

0 commit comments

Comments
 (0)