Uploaded image for project: 'Erlang/OTP'
  1. Erlang/OTP
  2. ERL-370

Check for null bytes in binaries / strings when opening files

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 19.1.1
    • Fix Version/s: OTP-21.0
    • Component/s: erts, kernel, stdlib
    • Labels:
      None

      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

        Hide
        rickard Rickard Green added a comment -

        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, stdlib
                       Related Id(s): ERL-370
         
                       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.
         
                       Note that functionality in modules manipulating
                       filenames 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 the newly introduced filename:validate/1.
         
                       WARNING: 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.
        

        Show
        rickard Rickard Green added a comment - 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, stdlib Related Id(s): ERL-370   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.   Note that functionality in modules manipulating filenames 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 the newly introduced filename:validate/1.   WARNING: 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.
        Hide
        josevalim 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?

        Show
        josevalim 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?
        Hide
        rickard 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.

        Show
        rickard 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.
        Hide
        rickard 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...

        Show
        rickard 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...
        Hide
        rickard 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

        Show
        rickard 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
        Hide
        rickard 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.
        

        Show
        rickard 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.

          People

          • Assignee:
            rickard Rickard Green
            Reporter:
            josevalim José Valim
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development