From eae496a72e270fd7af411714738e99a7fadfd19b Mon Sep 17 00:00:00 2001 From: Rickard Green Date: Wed, 6 Sep 2017 17:00:14 +0200 Subject: Don't allow null in filenames --- erts/emulator/beam/erl_bif_port.c | 2 +- erts/emulator/beam/erl_unicode.c | 58 ++++++++++++++++++++++++--- erts/emulator/beam/global.h | 2 +- erts/preloaded/src/erts.app.src | 2 +- lib/kernel/doc/src/file.xml | 38 ++++++++++++++++-- lib/kernel/src/kernel.app.src | 2 +- lib/kernel/test/file_name_SUITE.erl | 2 + lib/stdlib/doc/src/filelib.xml | 24 +++++++++++ lib/stdlib/doc/src/filename.xml | 78 +++++++++++++++++++++++++++++++++++- lib/stdlib/doc/src/unicode_usage.xml | 4 +- lib/stdlib/src/filename.erl | 70 ++++++++++++++++++++++++++++++++ lib/stdlib/src/stdlib.app.src | 2 +- lib/stdlib/test/filename_SUITE.erl | 57 +++++++++++++++++++++++++- 13 files changed, 323 insertions(+), 18 deletions(-) diff --git a/erts/emulator/beam/erl_bif_port.c b/erts/emulator/beam/erl_bif_port.c index ff03151619..1121a450cd 100644 --- a/erts/emulator/beam/erl_bif_port.c +++ b/erts/emulator/beam/erl_bif_port.c @@ -1084,7 +1084,7 @@ static byte* convert_environment(Process* p, Eterm env) goto done; } - if ((size = erts_native_filename_need(all,encoding)) < 0) { + if ((size = erts_native_filename_need(all, encoding, 1)) < 0) { goto done; } diff --git a/erts/emulator/beam/erl_unicode.c b/erts/emulator/beam/erl_unicode.c index 2d1d1443a7..efd2ca3db2 100644 --- a/erts/emulator/beam/erl_unicode.c +++ b/erts/emulator/beam/erl_unicode.c @@ -1988,7 +1988,7 @@ char *erts_convert_filename_to_encoding(Eterm name, char *statbuf, size_t statbu is_list(name) || (allow_empty && is_nil(name))) { Sint need; - if ((need = erts_native_filename_need(name,encoding)) < 0) { + if ((need = erts_native_filename_need(name, encoding, 0)) < 0) { return NULL; } if (encoding == ERL_FILENAME_WIN_WCHAR) { @@ -2152,12 +2152,13 @@ Eterm erts_convert_native_to_filename(Process *p, byte *bytes) } -Sint erts_native_filename_need(Eterm ioterm, int encoding) +Sint erts_native_filename_need(Eterm ioterm, int encoding, int allow_null) { Eterm *objp; Eterm obj; DECLARE_ESTACK(stack); Sint need = 0; + int seen_null = 0; if (is_atom(ioterm)) { Atom* ap; @@ -2194,6 +2195,24 @@ Sint erts_native_filename_need(Eterm ioterm, int encoding) default: need = -1; } + if (!allow_null) { + /* + * Do not allow null in + * the middle of filenames + */ + if (need > 0) { + byte *name = ap->name; + int len = ap->len; + for (i = 0; i < len; i++) { + if (name[i] == 0) + seen_null = 1; + else if (seen_null) { + need = -1; + break; + } + } + } + } DESTROY_ESTACK(stack); return need; } @@ -2224,6 +2243,18 @@ L_Again: /* Restart with sublist, old listend was pushed on stack */ if (is_small(obj)) { /* Always small */ for(;;) { Uint x = unsigned_val(obj); + if (!allow_null) { + /* + * Do not allow null in + * the middle of filenames + */ + if (x == 0) + seen_null = 1; + else if (seen_null) { + DESTROY_ESTACK(stack); + return ((Sint) -1); + } + } switch (encoding) { case ERL_FILENAME_LATIN1: if (x > 255) { @@ -2515,6 +2546,7 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1) BIF_ERROR(BIF_P,BADARG); } if (is_binary(BIF_ARG_1)) { + int seen_null = 0; byte *temp_alloc = NULL; byte *bytes; byte *err_pos; @@ -2524,10 +2556,18 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1) size = binary_size(BIF_ARG_1); bytes = erts_get_aligned_binary_bytes(BIF_ARG_1, &temp_alloc); if (encoding != ERL_FILENAME_WIN_WCHAR) { + Uint i; /*Add 0 termination only*/ bin_term = new_binary(BIF_P, NULL, size+1); bin_p = binary_bytes(bin_term); - memcpy(bin_p,bytes,size); + for (i = 0; i < size; i++) { + /* Don't allow null in the middle of filenames... */ + if (bytes[i] == 0) + seen_null = 1; + else if (seen_null) + goto bin_name_error; + bin_p[i] = bytes[i]; + } bin_p[size]=0; erts_free_aligned_binary_bytes(temp_alloc); BIF_RET(bin_term); @@ -2541,6 +2581,11 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1) bin_term = new_binary(BIF_P, 0, (size+1)*2); bin_p = binary_bytes(bin_term); while (size--) { + /* Don't allow null in the middle of filenames... */ + if (*bytes == 0) + seen_null = 1; + else if (seen_null) + goto bin_name_error; *bin_p++ = *bytes++; *bin_p++ = 0; } @@ -2558,11 +2603,14 @@ BIF_RETTYPE prim_file_internal_name2native_1(BIF_ALIST_1) bin_p[num_chars*2+1] = 0; erts_free_aligned_binary_bytes(temp_alloc); BIF_RET(bin_term); + bin_name_error: + erts_free_aligned_binary_bytes(temp_alloc); + BIF_ERROR(BIF_P,BADARG); } /* binary */ - if ((need = erts_native_filename_need(BIF_ARG_1,encoding)) < 0) { - BIF_ERROR(BIF_P,BADARG); + if ((need = erts_native_filename_need(BIF_ARG_1, encoding, 0)) < 0) { + BIF_ERROR(BIF_P,BADARG); } if (encoding == ERL_FILENAME_WIN_WCHAR) { need += 2; diff --git a/erts/emulator/beam/global.h b/erts/emulator/beam/global.h index 182d3aa44e..ddf7f03265 100644 --- a/erts/emulator/beam/global.h +++ b/erts/emulator/beam/global.h @@ -1253,7 +1253,7 @@ void erts_init_unicode(void); Sint erts_unicode_set_loop_limit(Sint limit); void erts_native_filename_put(Eterm ioterm, int encoding, byte *p) ; -Sint erts_native_filename_need(Eterm ioterm, int encoding); +Sint erts_native_filename_need(Eterm ioterm, int encoding, int allow_null); void erts_copy_utf8_to_utf16_little(byte *target, byte *bytes, int num_chars); int erts_analyze_utf8(byte *source, Uint size, byte **err_pos, Uint *num_chars, int *left); diff --git a/erts/preloaded/src/erts.app.src b/erts/preloaded/src/erts.app.src index 7ab06164b4..beb29a7c89 100644 --- a/erts/preloaded/src/erts.app.src +++ b/erts/preloaded/src/erts.app.src @@ -37,7 +37,7 @@ {registered, []}, {applications, []}, {env, []}, - {runtime_dependencies, ["stdlib-3.0", "kernel-5.0", "sasl-3.0.1"]} + {runtime_dependencies, ["stdlib-3.4.3", "kernel-5.4.1", "sasl-3.0.1"]} ]}. %% vim: ft=erlang diff --git a/lib/kernel/doc/src/file.xml b/lib/kernel/doc/src/file.xml index b674b3ca93..593bee74fe 100644 --- a/lib/kernel/doc/src/file.xml +++ b/lib/kernel/doc/src/file.xml @@ -59,7 +59,7 @@ terminal supports UTF-8, otherwise latin1. The default can be overridden using +fnl (to force latin1 mode) or +fnu (to force utf8 mode) when starting - erts:erl.

+ erl.

On operating systems with transparent naming, files can be inconsistently named, for example, some files are encoded in UTF-8 while @@ -81,6 +81,22 @@

See also section Notes About Raw Filenames in the STDLIB User's Guide.

+

+ File operations used to accept filenames containing + null characters (integer value zero). This caused + the name to be truncated at the first null character. + Filenames containing null characters inside the filename + are now rejected and will cause primitive + file operations fail. +

+

+ Currently null characters at the end of the filename + will be accepted by primitive file operations. Such + filenames are however still documented as invalid. The + implementation will also change in the future and + reject such filenames. +

+ @@ -96,9 +112,21 @@ + +

+ See also the documentation of the + name_all() type. +

+ + +

+ See also the documentation of the + name_all() type. +

+ @@ -112,21 +140,23 @@

If VM is in Unicode filename mode, string() and char() - are allowed to be > 255. + are allowed to be > 255. See also the documentation of the + name_all() type.

-

If VM is in Unicode filename mode, string() and char() +

If VM is in Unicode filename mode, characters are allowed to be > 255. RawFilename is a filename not subject to Unicode translation, meaning that it can contain characters not conforming to the Unicode encoding expected from the file system (that is, non-UTF-8 characters although the VM is started - in Unicode filename mode). + in Unicode filename mode). Null characters (integer value zero) + are not allowed in filenames (not even at the end).

diff --git a/lib/kernel/src/kernel.app.src b/lib/kernel/src/kernel.app.src index 2a88cc7e26..b5e5f8eb73 100644 --- a/lib/kernel/src/kernel.app.src +++ b/lib/kernel/src/kernel.app.src @@ -120,6 +120,6 @@ {applications, []}, {env, [{error_logger, tty}]}, {mod, {kernel, []}}, - {runtime_dependencies, ["erts-9.1", "stdlib-3.4", "sasl-3.0"]} + {runtime_dependencies, ["erts-9.1.1", "stdlib-3.4.3", "sasl-3.0"]} ] }. diff --git a/lib/kernel/test/file_name_SUITE.erl b/lib/kernel/test/file_name_SUITE.erl index 899102c908..f23529fec9 100644 --- a/lib/kernel/test/file_name_SUITE.erl +++ b/lib/kernel/test/file_name_SUITE.erl @@ -302,7 +302,9 @@ check_normal(Mod) -> {ok, BC} = Mod:read(FD,1024), ok = file:close(FD) end || {regular,Name,Content} <- NormalDir ], + {error, badarg} = Mod:rename("fil1\0tmp_fil2","tmp_fil1"), Mod:rename("fil1","tmp_fil1"), + {error, badarg} = Mod:read_file("tmp_fil1\0.txt"), {ok, <<"fil1">>} = Mod:read_file("tmp_fil1"), {error,enoent} = Mod:read_file("fil1"), Mod:rename("tmp_fil1","fil1"), diff --git a/lib/stdlib/doc/src/filelib.xml b/lib/stdlib/doc/src/filelib.xml index 80c4acffdb..57c4348745 100644 --- a/lib/stdlib/doc/src/filelib.xml +++ b/lib/stdlib/doc/src/filelib.xml @@ -45,6 +45,30 @@

For more information about raw filenames, see the file module.

+ + +

+ Functionality in this module generally assumes valid input and + does not necessarily fail on input that does not use a valid + encoding. You can validate the encoding of a filename using + filename:validate/1. +

+

+ File operations used to accept filenames containing + null characters (integer value zero). This caused + the name to be truncated at the first null character. + Filenames containing null characters inside the filename + are now rejected and will cause primitive + file operations fail. +

+ +

+ Currently null characters at the end of the filename + will be accepted by primitive file operations. Such + filenames are however still documented as invalid. The + implementation will also change in the future and + reject such filenames. +

diff --git a/lib/stdlib/doc/src/filename.xml b/lib/stdlib/doc/src/filename.xml index 14fd5ef787..b6028fc066 100644 --- a/lib/stdlib/doc/src/filename.xml +++ b/lib/stdlib/doc/src/filename.xml @@ -46,7 +46,10 @@ filename by removing redundant directory separators, use join/1.

-

The module supports raw filenames in the way that if a binary is +

+ The module supports + raw + filenames in the way that if a binary is present, or the filename cannot be interpreted according to the return value of file:native_name_encoding/0, a raw filename is also @@ -56,6 +59,30 @@ (the join operation is performed of course). For more information about raw filenames, see the file module.

+ + +

+ Functionality in this module generally assumes valid input and + does not necessarily fail on input that does not use a valid + encoding. You can validate the encoding of a filename using + filename:validate/1. +

+

+ File operations used to accept filenames containing + null characters (integer value zero). This caused + the name to be truncated at the first null character. + Filenames containing null characters inside the filename + are now rejected and will cause primitive + file operations fail. +

+ +

+ Currently null characters at the end of the filename + will be accepted by primitive file operations. Such + filenames are however still documented as invalid. The + implementation will also change in the future and + reject such filenames. +

@@ -555,6 +582,55 @@ unsafe ["a:/","msdev","include"] + + + + Validate encoding of filename + +

+ Validates filename encoding. Returns true if + FileName has a valid encoding; + otherwise, returns false. +

+ + Ordinary Filename + +

+ Type: FileName = file:name() +

+

+ Validates encoding against the + native file + name encoding, and the + capabilities of the operating system used. + Regardless of configuration and OS, null + characters (integer value zero) will be + rejected by the validation (even when only + present at the end of the filename). +

+ + Raw + Filename + +

+ Type: FileName = binary() +

+

+ The encoding will not be interpreted, but + null bytes (integer value zero) will be + rejected by the validation (even when only + present at the end of the filename). +

+ + +

+ For information on filename encoding see the documentation + of unicode filenames in + STDLIB + Users Guide ➜ Using Unicode in Erlang ➜ Unicode Filenames. +

+ + diff --git a/lib/stdlib/doc/src/unicode_usage.xml b/lib/stdlib/doc/src/unicode_usage.xml index 26dc46719e..ff1f864e22 100644 --- a/lib/stdlib/doc/src/unicode_usage.xml +++ b/lib/stdlib/doc/src/unicode_usage.xml @@ -719,8 +719,8 @@ Eshell V5.10.1 (abort with ^G)
- Unicode Filenames + Unicode Filenames

Most modern operating systems support Unicode filenames in some way. There are many different ways to do this and Erlang by default treats the different approaches differently:

@@ -855,8 +855,8 @@ Eshell V5.10.1 (abort with ^G)
- Notes About Raw Filenames + Notes About Raw Filenames

Raw filenames were introduced together with Unicode filename support in ERTS 5.8.2 (Erlang/OTP R14B01). The reason "raw filenames" were introduced in the system was diff --git a/lib/stdlib/src/filename.erl b/lib/stdlib/src/filename.erl index 9bf4290916..1c3ab6d274 100644 --- a/lib/stdlib/src/filename.erl +++ b/lib/stdlib/src/filename.erl @@ -41,6 +41,7 @@ safe_relative_path/1]). -export([find_src/1, find_src/2]). % deprecated -export([basedir/2, basedir/3]). +-export([validate/1]). %% Undocumented and unsupported exports. -export([append/2]). @@ -1135,3 +1136,72 @@ basedir_os_type() -> {win32,_} -> windows; _ -> linux end. + +%% +%% validate/1 +%% + +-spec validate(FileName) -> boolean() when + FileName :: file:name_all(). + +validate(FileName) when is_binary(FileName) -> + %% Raw filename... + validate_bin(FileName); +validate(FileName) when is_list(FileName); + is_atom(FileName) -> + validate_list(FileName, + file:native_name_encoding(), + os:type()). + +validate_list(FileName, Enc, Os) -> + try + true = validate_list(FileName, Enc, Os, 0) > 0 + catch + _ : _ -> false + end. + +validate_list([], _Enc, _Os, Chars) -> + Chars; +validate_list(C, Enc, Os, Chars) when is_integer(C) -> + validate_char(C, Enc, Os), + Chars+1; +validate_list(A, Enc, Os, Chars) when is_atom(A) -> + validate_list(atom_to_list(A), Enc, Os, Chars); +validate_list([H|T], Enc, Os, Chars) -> + NewChars = validate_list(H, Enc, Os, Chars), + validate_list(T, Enc, Os, NewChars). + +%% C is always an integer... +% validate_char(C, _, _) when not is_integer(C) -> +% throw(invalid); +validate_char(C, _, _) when C < 1 -> + throw(invalid); %% No negative or null characters... +validate_char(C, latin1, _) when C > 255 -> + throw(invalid); +validate_char(C, utf8, _) when C >= 16#110000 -> + throw(invalid); +validate_char(C, utf8, {win32, _}) when C > 16#ffff -> + throw(invalid); %% invalid win wchar... +validate_char(_C, utf8, {win32, _}) -> + ok; %% Range below is accepted on windows... +validate_char(C, utf8, _) when 16#D800 =< C, C =< 16#DFFF -> + throw(invalid); %% invalid unicode range... +validate_char(_, _, _) -> + ok. + +validate_bin(Bin) -> + %% Raw filename. That is, we do not interpret + %% the encoding, but we still do not accept + %% null characters... + try + true = validate_bin(Bin, 0) > 0 + catch + _ : _ -> false + end. + +validate_bin(<<>>, Bs) -> + Bs; +validate_bin(<<0, _Rest/binary>>, _Bs) -> + throw(invalid); %% No null characters allowed... +validate_bin(<<_B, Rest/binary>>, Bs) -> + validate_bin(Rest, Bs+1). diff --git a/lib/stdlib/src/stdlib.app.src b/lib/stdlib/src/stdlib.app.src index 3c449d3cb9..41c89270aa 100644 --- a/lib/stdlib/src/stdlib.app.src +++ b/lib/stdlib/src/stdlib.app.src @@ -107,7 +107,7 @@ dets]}, {applications, [kernel]}, {env, []}, - {runtime_dependencies, ["sasl-3.0","kernel-5.0","erts-9.0","crypto-3.3", + {runtime_dependencies, ["sasl-3.0","kernel-5.4.1","erts-9.1.1","crypto-3.3", "compiler-5.0"]} ]}. diff --git a/lib/stdlib/test/filename_SUITE.erl b/lib/stdlib/test/filename_SUITE.erl index fc77593bb8..4c82ec1c22 100644 --- a/lib/stdlib/test/filename_SUITE.erl +++ b/lib/stdlib/test/filename_SUITE.erl @@ -30,6 +30,7 @@ -export([pathtype_bin/1,rootname_bin/1,split_bin/1]). -export([t_basedir_api/1, t_basedir_xdg/1, t_basedir_windows/1]). -export([safe_relative_path/1]). +-export([validate/1]). -include_lib("common_test/include/ct.hrl"). @@ -43,7 +44,8 @@ all() -> absname_bin, absname_bin_2, {group,p}, t_basedir_xdg, t_basedir_windows, - safe_relative_path]. + safe_relative_path, + validate]. groups() -> [{p, [parallel], @@ -1011,3 +1013,56 @@ basedir_xdg_def(Type,Home,Name) -> Dir <- ["/usr/local/share/","/usr/share/"]]; site_config -> [filename:join(["/etc/xdg",Name])] end. + +validate(Config) when is_list(Config) -> + true = filename:validate(blipp), + false = filename:validate('bli\0pp'), + false = filename:validate('blipp\0'), + true = filename:validate("blipp"), + false = filename:validate("bli"++[0]++"pp"), + false = filename:validate("blipp"++[0]), + true = filename:validate(["one ", blipp, "blopp"]), + false = filename:validate(["one ", 'bli\0pp', "blopp"]), + false = filename:validate(["one ", 'blipp\0', "blopp"]), + false = filename:validate(["one ", 'blipp', "blopp\0"]), + false = filename:validate([0]), + false = filename:validate([]), + false = filename:validate([[[]],[[[[],[[[[[[[[]]], '', [[[[[]]]]]]]]]]]]]]), + false = filename:validate([16#110000]), + false = filename:validate([16#110001]), + false = filename:validate([16#110000*2]), + case file:native_name_encoding() of + latin1 -> + true = filename:validate(lists:seq(1, 255)), + false = filename:validate([256]); + utf8 -> + true = filename:validate(lists:seq(1, 16#D7FF)), + true = filename:validate(lists:seq(16#E000, 16#FFFF)), + true = filename:validate([16#FFFF]), + case os:type() of + {win32, _} -> + false = filename:validate([16#10000]), + true = filename:validate(lists:seq(16#D800,16#DFFF)); + _ -> + true = filename:validate([16#10000]), + true = filename:validate([16#10FFFF]), + lists:foreach(fun (C) -> + false = filename:validate([C]) + end, + lists:seq(16#D800,16#DFFF)) + end + + end, + true = filename:validate(<<1,17,255>>), + false = filename:validate(<<1,0,17,255>>), + false = filename:validate(<<1,17,255,0>>), + false = filename:validate(<<>>), + lists:foreach(fun (N) -> + true = filename:validate(N) + end, + code:get_path()), + ok. + + + + -- cgit v1.2.3