|
| 1 | +From 10608879c81332af2d3c00db61ee173c93c1ea4e Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?Lukas=20Backstr=C3=B6m?= <lukas@erlang.org> |
| 3 | +Date: Tue, 27 May 2025 21:50:01 +0200 |
| 4 | +Subject: [PATCH] stdlib: Properly sanatize filenames when (un)zipping |
| 5 | + |
| 6 | +Upstream Patch Link: https://github.com/erlang/otp/pull/9941/commits/10608879c81332af2d3c00db61ee173c93c1ea4e.patch |
| 7 | + |
| 8 | +According to the Zip APPNOTE filenames "MUST NOT contain a drive or |
| 9 | +device letter, or a leading slash.". So we strip those when zipping |
| 10 | +and unzipping. |
| 11 | +--- |
| 12 | + lib/stdlib/src/zip.erl | 21 ++++++++++++++---- |
| 13 | + lib/stdlib/test/zip_SUITE.erl | 40 ++++++++++++++++++++++++++++------- |
| 14 | + 2 files changed, 49 insertions(+), 12 deletions(-) |
| 15 | + |
| 16 | +diff --git a/lib/stdlib/src/zip.erl b/lib/stdlib/src/zip.erl |
| 17 | +index 0809dbb492b4..b75055024ca3 100644 |
| 18 | +--- a/lib/stdlib/src/zip.erl |
| 19 | ++++ b/lib/stdlib/src/zip.erl |
| 20 | +@@ -833,12 +833,12 @@ get_filename({Name, _}, Type) -> |
| 21 | + get_filename({Name, _, _}, Type) -> |
| 22 | + get_filename(Name, Type); |
| 23 | + get_filename(Name, regular) -> |
| 24 | +- Name; |
| 25 | ++ sanitize_filename(Name); |
| 26 | + get_filename(Name, directory) -> |
| 27 | + %% Ensure trailing slash |
| 28 | + case lists:reverse(Name) of |
| 29 | +- [$/ | _Rev] -> Name; |
| 30 | +- Rev -> lists:reverse([$/ | Rev]) |
| 31 | ++ [$/ | _Rev] -> sanitize_filename(Name); |
| 32 | ++ Rev -> sanitize_filename(lists:reverse([$/ | Rev])) |
| 33 | + end. |
| 34 | + |
| 35 | + add_cwd(_CWD, {_Name, _} = F) -> F; |
| 36 | +@@ -1550,12 +1550,25 @@ check_dir_level([_Dir | Parts], Level) -> |
| 37 | + get_file_name_extra(FileNameLen, ExtraLen, B, GPFlag) -> |
| 38 | + try |
| 39 | + <<BFileName:FileNameLen/binary, BExtra:ExtraLen/binary>> = B, |
| 40 | +- {binary_to_chars(BFileName, GPFlag), BExtra} |
| 41 | ++ {sanitize_filename(binary_to_chars(BFileName, GPFlag)), BExtra} |
| 42 | + catch |
| 43 | + _:_ -> |
| 44 | + throw(bad_file_header) |
| 45 | + end. |
| 46 | + |
| 47 | ++sanitize_filename(Filename) -> |
| 48 | ++ case filename:pathtype(Filename) of |
| 49 | ++ relative -> Filename; |
| 50 | ++ _ -> |
| 51 | ++ %% With absolute or volumerelative, we drop the prefix and rejoin |
| 52 | ++ %% the path to create a relative path |
| 53 | ++ Relative = filename:join(tl(filename:split(Filename))), |
| 54 | ++ error_logger:format("Illegal absolute path: ~ts, converting to ~ts~n", |
| 55 | ++ [Filename, Relative]), |
| 56 | ++ relative = filename:pathtype(Relative), |
| 57 | ++ Relative |
| 58 | ++ end. |
| 59 | ++ |
| 60 | + %% get compressed or stored data |
| 61 | + get_z_data(?DEFLATED, In0, FileName, CompSize, Input, Output, OpO, Z) -> |
| 62 | + ok = zlib:inflateInit(Z, -?MAX_WBITS), |
| 63 | +diff --git a/lib/stdlib/test/zip_SUITE.erl b/lib/stdlib/test/zip_SUITE.erl |
| 64 | +index 97e5c660dd96..1edf6c1067e7 100644 |
| 65 | +--- a/lib/stdlib/test/zip_SUITE.erl |
| 66 | ++++ b/lib/stdlib/test/zip_SUITE.erl |
| 67 | +@@ -22,7 +22,7 @@ |
| 68 | + -export([all/0, suite/0,groups/0,init_per_suite/1, end_per_suite/1, |
| 69 | + init_per_group/2,end_per_group/2, borderline/1, atomic/1, |
| 70 | + bad_zip/1, unzip_from_binary/1, unzip_to_binary/1, |
| 71 | +- zip_to_binary/1, |
| 72 | ++ zip_to_binary/1, sanitize_filenames/1, |
| 73 | + unzip_options/1, zip_options/1, list_dir_options/1, aliases/1, |
| 74 | + openzip_api/1, zip_api/1, open_leak/1, unzip_jar/1, |
| 75 | + unzip_traversal_exploit/1, |
| 76 | +@@ -40,7 +40,8 @@ all() -> |
| 77 | + unzip_to_binary, zip_to_binary, unzip_options, |
| 78 | + zip_options, list_dir_options, aliases, openzip_api, |
| 79 | + zip_api, open_leak, unzip_jar, compress_control, foldl, |
| 80 | +- unzip_traversal_exploit,fd_leak,unicode,test_zip_dir]. |
| 81 | ++ unzip_traversal_exploit,fd_leak,unicode,test_zip_dir, |
| 82 | ++ sanitize_filenames]. |
| 83 | + |
| 84 | + groups() -> |
| 85 | + []. |
| 86 | +@@ -90,22 +91,27 @@ borderline_test(Size, TempDir) -> |
| 87 | + {ok, Archive} = zip:zip(Archive, [Name]), |
| 88 | + ok = file:delete(Name), |
| 89 | + |
| 90 | ++ RelName = filename:join(tl(filename:split(Name))), |
| 91 | ++ |
| 92 | + %% Verify listing and extracting. |
| 93 | + {ok, [#zip_comment{comment = []}, |
| 94 | +- #zip_file{name = Name, |
| 95 | ++ #zip_file{name = RelName, |
| 96 | + info = Info, |
| 97 | + offset = 0, |
| 98 | + comp_size = _}]} = zip:list_dir(Archive), |
| 99 | + Size = Info#file_info.size, |
| 100 | +- {ok, [Name]} = zip:extract(Archive, [verbose]), |
| 101 | ++ TempRelName = filename:join(TempDir, RelName), |
| 102 | ++ {ok, [TempRelName]} = zip:extract(Archive, [verbose, {cwd, TempDir}]), |
| 103 | + |
| 104 | +- %% Verify contents of extracted file. |
| 105 | +- {ok, Bin} = file:read_file(Name), |
| 106 | +- true = match_byte_list(X0, binary_to_list(Bin)), |
| 107 | ++ %% Verify that absolute file was not created |
| 108 | ++ {error, enoent} = file:read_file(Name), |
| 109 | + |
| 110 | ++ %% Verify that relative contents of extracted file. |
| 111 | ++ {ok, Bin} = file:read_file(TempRelName), |
| 112 | ++ true = match_byte_list(X0, binary_to_list(Bin)), |
| 113 | + |
| 114 | + %% Verify that Unix zip can read it. (if we have a unix zip that is!) |
| 115 | +- zipinfo_match(Archive, Name), |
| 116 | ++ zipinfo_match(Archive, RelName), |
| 117 | + |
| 118 | + ok. |
| 119 | + |
| 120 | +@@ -1054,3 +1060,21 @@ run_command(Command, Args) -> |
| 121 | + end |
| 122 | + end)(). |
| 123 | + |
| 124 | ++sanitize_filenames(Config) -> |
| 125 | ++ RootDir = proplists:get_value(priv_dir, Config), |
| 126 | ++ TempDir = filename:join(RootDir, "borderline"), |
| 127 | ++ ok = file:make_dir(TempDir), |
| 128 | ++ |
| 129 | ++ %% Create a zip archive /tmp/absolute in it |
| 130 | ++ %% This file was created using the command below on Erlang/OTP 28.0 |
| 131 | ++ %% 1> rr(file), {ok, {_, Bin}} = zip:zip("absolute.zip", [{"/tmp/absolute",<<>>,#file_info{ type=regular, mtime={{1970,1,1},{0,0,0}}, size=0 }}], [memory]), rp(base64:encode(Bin)). |
| 132 | ++ AbsZip = base64:decode(<<"UEsDBBQAAAAAAAAAIewAAAAAAAAAAAAAAAANAAAAL3RtcC9hYnNvbHV0ZVBLAQIUAxQAAAAAAAAAIewAAAAAAAAAAAAAAAANAAAAAAAAAAAAAACkAQAAAAAvdG1wL2Fic29sdXRlUEsFBgAAAAABAAEAOwAAACsAAAAAAA==">>), |
| 133 | ++ Archive = filename:join(TempDir, "absolute.zip"), |
| 134 | ++ ok = file:write_file(Archive, AbsZip), |
| 135 | ++ |
| 136 | ++ TmpAbs = filename:join([TempDir, "tmp", "absolute"]), |
| 137 | ++ {ok, [TmpAbs]} = zip:unzip(Archive, [verbose, {cwd, TempDir}]), |
| 138 | ++ {error, enoent} = file:read_file("/tmp/absolute"), |
| 139 | ++ {ok, <<>>} = file:read_file(TmpAbs), |
| 140 | ++ |
| 141 | ++ ok. |
| 142 | +\ No newline at end of file |
0 commit comments