Details
Bug
Major
Description
Currently file:read_file and all other operations that accept a file system path representing file, including in other modules such as prim_file, silently accept and discard the remaining of a string / binary when a null byte is present. For example, if I have a "README.md" file in the current directory, I can read it with the commands below:
1> file:read_file("README.md\0.txt").
{ok, ...}2> file:read_file(<<"README.md\0.txt">>).
{ok, ...}
This can be a security issue in applications that perform operations based on the filename. For example, filename:extname/1 returns the extension name after the null byte:
3> filename:extension("README.md\0.txt").
".txt"
So an entity could be made to believe it is handling a file with extension .xxx while it is serving a file with extension .yyy.
While I don't believe this poises a security issue in OTP itself, I believe the platform would be safer if it raised when a string or binary with a null byte is given anytime we are interacting with the filesystem.
For some reference, Java, Python, Node and Ruby all raise if the string/binary contains a null byte. Haskell is the only language that does not in my initial analysis.
Activity
Thanks Rickard, this is great news!
I have recently discovered that many operations in the `os` module have the same issue, for example: `os:getenv("PATH\0FOO")`.
I would say we need to check for null bytes at least on getenv, putenv and find_executable. os:cmd/1 may also need sanitization of input as well as open_port/2, although those cases are probably more complicated.
Should I open up a new issue for those or is it being tracked elsewhere?
José Valim
added a comment - Thanks Rickard, this is great news!
I have recently discovered that many operations in the `os` module have the same issue, for example: `os:getenv("PATH\0FOO")`.
I would say we need to check for null bytes at least on getenv, putenv and find_executable. os:cmd/1 may also need sanitization of input as well as open_port/2, although those cases are probably more complicated.
Should I open up a new issue for those or is it being tracked elsewhere? All of these should have been fixed by these changes as well. I however forgot to mention environment variables in the release note and documentation, will update these. The same strategy applies to environment variables we allow null characters at the end (only) today, but will reject them in the future.
Rickard Green
added a comment - All of these should have been fixed by these changes as well. I however forgot to mention environment variables in the release note and documentation, will update these. The same strategy applies to environment variables we allow null characters at the end (only) today, but will reject them in the future. The env option of open_port/2 however still have issues 
will fix this. Please let me know if you find anything more with similar issues...
Rickard Green
added a comment - The env option of open_port/2 however still have issues will fix this. Please let me know if you find anything more with similar issues... This fix will unfortunately not be part of OTP 20.1.1 due to issues listed above, but will hopefully be part of OTP 20.1.2
Rickard Green
added a comment - This fix will unfortunately not be part of OTP 20.1.1 due to issues listed above, but will hopefully be part of OTP 20.1.2 I reverted the merge into the maint branch, and merged updated fixes into the master branch. That is, these fixes will be released in OTP 21.0. This since the changes might be viewed as potentially incompatible. Release note:
OTP-14543 Application(s): erts, kernel, stdlib
|
Related Id(s): ERL-370
|
|
|
*** POTENTIAL INCOMPATIBILITY ***
|
|
|
File operations used to accept filenames containing
|
null characters (integer value zero). This caused the
|
name to be truncated and in some cases arguments to
|
primitive operations to be mixed up. Filenames
|
containing null characters inside the filename are now
|
*rejected* and will cause primitive file operations to
|
fail.
|
|
|
Also environment variable operations used to accept
|
names and values of environment variables containing
|
null characters (integer value zero). This caused
|
operations to silently produce erroneous results.
|
Environment variable names and values containing null
|
characters inside the name or value are now *rejected*
|
and will cause environment variable operations to fail.
|
|
|
Primitive environment variable operations also used to
|
accept the $= character in environment variable names
|
causing various mixups. $= characters in environment
|
variable names are now also *rejected*.
|
|
|
Also os:cmd/1 now reject null characters inside its
|
command.
|
|
|
WARNING: Currently null characters at the end of
|
filenames, environment variable names and values will
|
be accepted by the primitive operations. Such
|
filenames, environment variable names and values are
|
however still documented as invalid. The implementation
|
will also change in the future and reject such
|
filenames, environment variable names and values.
|
Rickard Green
added a comment - I reverted the merge into the maint branch, and merged updated fixes into the master branch. That is, these fixes will be released in OTP 21.0. This since the changes might be viewed as potentially incompatible. Release note:
OTP-14543 Application(s): erts, kernel, stdlib
Related Id(s): ERL-370
*** POTENTIAL INCOMPATIBILITY ***
File operations used to accept filenames containing
null characters (integer value zero). This caused the
name to be truncated and in some cases arguments to
primitive operations to be mixed up. Filenames
containing null characters inside the filename are now
*rejected* and will cause primitive file operations to
fail.
Also environment variable operations used to accept
names and values of environment variables containing
null characters (integer value zero). This caused
operations to silently produce erroneous results.
Environment variable names and values containing null
characters inside the name or value are now *rejected*
and will cause environment variable operations to fail.
Primitive environment variable operations also used to
accept the $= character in environment variable names
causing various mixups. $= characters in environment
variable names are now also *rejected*.
Also os:cmd/1 now reject null characters inside its
command.
WARNING: Currently null characters at the end of
filenames, environment variable names and values will
be accepted by the primitive operations. Such
filenames, environment variable names and values are
however still documented as invalid. The implementation
will also change in the future and reject such
filenames, environment variable names and values.
I've now merged a fix for this into the maint and master branches. The fix will be released in the next patch (OTP 20.1.1).
Release note:
OTP-14543 Application(s): erts, kernel, stdlibRelated Id(s): ERL-370File operations used to accept filenames containingnull characters (integer value zero). This caused thename to be truncated at the first null character.Filenames containing null characters inside thefilename are now *rejected* and will cause primitivefile operations fail.Note that functionality in modules manipulatingfilenames generally assumes valid input and does notnecessarily fail on input that does not use a validencoding. You can validate the encoding of a filenameusing the newly introduced filename:validate/1.WARNING: Currently null characters at the end of thefilename will be accepted by primitive file operations.Such filenames are however still documented as invalid.The implementation will also change in the future andreject such filenames.