Static and session security fixes for Plug

security
plug
Tags: #<Tag:0x00007fbcac59c8b8> #<Tag:0x00007fbcac59c750>

#1

Hello everyone,

Two vulnerabilities have been disclosed to Plug. Applications that provide file uploading functionality to a local filesystem are advised to upgrade immediately. Upgrade is also recommended for any other application that uses Plug, to ensure they follow the latest and best security practices.

We have released new Plug versions v1.0.4, v1.1.7, v1.2.3 and v1.3.2. If you can’t upgrade immediately, we also include fixes you can directly add to your applications.

Null Byte Injection in Plug.Static

Plug.Static is used for serving static assets, and is vulnerable to null byte injection. If file upload functionality is provided, this can allow users to bypass filetype restrictions.

We recommend all applications that provide file upload functionality and serve those uploaded files locally with Plug.Static to upgrade immediately or include the fix below. If uploaded files are rather stored and served from S3 or any other cloud storage, you are not affected.

  • Versions affected: Plug v1.3.1, v1.3.0, v1.2.2, v1.2.1, v1.2.0, v1.1.6, v1.1.5, v1.1.4, v1.1.3, v1.1.2, v1.1.1, v1.1.0, v1.0.3, v1.0.2, v1.0.1, v1.0.0
  • Versions fixed: Plug v1.3.2+, v1.2.3+, v1.1.7+, v1.0.4+
  • Author: Griffin Byatt <griffin.byatt[at]nccgroup[dot]trust>
  • Thanks to: Chip Durland, Raviv Cohen and Matthew Diaz

Temporary fix

Identify where plug Plug.Static is invoked in your application (in a Phoenix application this means the MyApp.Endpoint) and add the following lines before plug Plug.Static:

plug :safe_plug_static

defp safe_plug_static(conn, _) do
  Enum.any?(conn.path_info, &URI.decode(&1) =~ <<0>>) && raise "invalid path"
  conn
end

Arbitrary Code Execution in Cookie Serialization

The default serialization used by Plug session may result in code execution in certain situations. Keep in mind, however, the session cookie is signed and this attack can only be exploited if the attacker has access to your secret key as well as your signing/encryption salts. We recommend users to change their secret key base and salts if they suspect they have been leaked, regardless of this vulnerability.

  • Versions affected: Plug v1.3.1, v1.3.0, v1.2.2, v1.2.1, v1.2.0, v1.1.6, v1.1.5, v1.1.4, v1.1.3, v1.1.2, v1.1.1, v1.1.0, v1.0.3, v1.0.2, v1.0.1, v1.0.0
  • Versions fixed: Plug v1.3.2+, v1.2.3+, v1.1.7+, v1.0.4+
  • Author: Griffin Byatt <griffin.byatt[at]nccgroup[dot]trust>
  • Thanks to: Matthew Diaz

Temporary fix

If you can’t upgrade immediately, you can temporarily protect yourself by using a custom serializer Plug.Session serializer.

plug Plug.Session, serializer: MyApp.SafeSerializer

Where MyApp.SafeSerializer is defined as:

defmodule SafeSerializer do
  def encode(term) do
    {:ok, :erlang.term_to_binary(term)}
  end

  def decode(binary) do
    try do
      {:ok, safe_terms(:erlang.binary_to_term(binary))}
    rescue
      _ -> :error
    end
  end

  defp safe_terms(list) when is_list(list) do
    for item <- list, do: safe_terms(item)
    list
  end
  defp safe_terms(tuple) when is_tuple(tuple) do
    safe_tuple(tuple, tuple_size(tuple))
    tuple
  end
  defp safe_terms(map) when is_map(map) do
    for {key, value} <- map do
      safe_terms(key)
      safe_terms(value)
    end
    map
  end
  defp safe_terms(other) when is_atom(other) or is_number(other) or is_binary(other) or
                              is_pid(other) or is_reference(other) do
    other
  end
  defp safe_terms(other) do
    raise ArgumentError, "cannot deserialize #{inspect other}, the term is not safe for deserialization"
  end

  defp safe_tuple(tuple, 0), do: :ok
  defp safe_tuple(tuple, n), do
    safe_terms(:erlang.element(n, tuple))
    safe_tuple(tuple, n - 1)
  end
end

Final remarks

We want to thank the NCC Group for reporting those vulnerabilities.

NCC Group is a global expert in cyber security and risk mitigation, working with businesses to protect their brand, value and reputation against the ever-evolving threat landscape. Our Security Consulting services leverage our extensive knowledge of current security vulnerabilities, penetration testing techniques and software development best practices to enable organizations to secure their applications against ever-present threats. At NCC Group we can boast unrivaled talent and recognized world-class security expertise. Bringing together best in class security consultancies iSEC Partners, Intrepidus Group, Matasano, NCC Group and NGS we have created one of the largest, most respected security consultancies in the world.


#2

#3

Thanks for staying on top of things, much appreciated! I continue being impressed by you guys.


#4

So, as a learning tool, if I get it it means that:

  1. Attacker could alter the cookie to include serialized function, that would get de-serialized and executed during cookie de-serialization? Provided they can keep the cookie cryptographic signature at the same time.

  2. Attacker could fetch a *.db file instead of *.ico file. Provided the secret file is somehow located in priv/static, and programmer specified :only, or :only_matching Plug.Static options. So fetching /assets/secret.db%00%favicon.ico would return secret.db file.

Looking at Plug code I can see the 2) could really happen. Although it would mean the programmer is doing something weird already, like putting the secret.db file where it should not have been.

I can’t see how 1) could be exploited unless the programmer actually executes de-serialized functions. I think :erlang.binary_to_term will not re-define any already existing modules/functions. The only way to exploit it would be that the programmer does execute such de-serialized function, again doing something pretty weird. Is that correct?


#5

The function is NOT executed during de-serialization.

You CANNOT bypass the :only and :only_matching checks with the null byte.

I cannot provide detailed information on how to exploit the attacks for now. In a week, once we have given developers enough time to upgrade, I will be glad to answer questions.


#6

Okay, even better. I don’t think either of the issues is really severe, both require that programmer does make some bad decisions in addition to patched issues.


#7

pure speculation below

Attacker could alter the cookie to include serialized function, that would get de-serialized and executed during cookie de-serialization? Provided they can keep the cookie cryptographic signature at the same time.

interesting point, looking forward to hear about what can be done here in a couple weeks. I’m scared of the scalable jwt concept though so I save auth cookies in the database and only process the ones I know I’ve generated

Attacker could fetch a *.db file instead of *.ico file. Provided the secret file is somehow located in priv/static, and programmer specified :only, or :only_matching Plug.Static options. So fetching /assets/secret.db%00%favicon.ico would return secret.db file.

they could maybe upload some bash scripts and the execute them doing something as the app user

I cannot provide detailed information on how to exploit the attacks for now. In a week, once we have given developers enough time to upgrade, I will be glad to answer questions.

@josevalim please do, I’d like to know more


#8

I think it is more likely a disallowed MIME type is sent to client. Like, you allow uploading/downloading PNG files but someone uploads a JAR file that gets automatically executed in the client’s browser.


#9

I would encourage the Phoenix team to create a CVE for this vulnerablity if they have not already.

https://cve.mitre.org/about/faqs.html


#10

Is there some sort of pipe that we can subscribe to for all sorts of Elixir security releases?


#11

Thoughts on something like gemnasium for this?


#12

I have released Plug v1.3.3, v1.2.4, v1.1.8, v1.0.5 that also allows structs, bitstrings and improper lists to be deserialized.

Finally, when this thread is 7 days old, I will provide more information about the vulnerabilities. So if you haven’t upgraded yet, please do so.


#13

Hi everyone,

Here is more information on the null byte issue, as reported by NCC Group:

The “static” plug, used to serve assets in Plug-based web frameworks, serves two primary functions: locating the requested file, and setting the response content type. The asset content type is set dynamically, using the Mime.from_path function. For example, a request for the file “images/phoenix.png” will result in a content type of “image/png.” However, if the request is updated to “images/phoenix.png%00.html,” the resulting content type will be set to “text/html.”

Some have mentioned the vulnerability could cause a “access control” issue, causing one user to see files permitted by other users, however I don’t believe that to be possible because:

  1. Plug.Static doesn’t provide any control access. All files are available to all users

  2. If you are implementing control access by plugging Plug.Static on an authenticated route, it is most likely that you are doing authentication based on the path prefix rather than the path suffix and any “weird character”, such as a null byte, would make authentication fail

However, it is important to mention that this vulnerability may affect you even if you don’t use null byte. A bug report has been filled on Erlang/OTP issues tracker.

I will provide more information about the other vulnerability soon.


#14

The access control issue @josevalim mentions is something I described in this post. I agree that most sites would probably implement sharing at the directory level, but some cloud syncing services have more fine-grained access control, for example for use in an enterprise environment.

Still, even if the example is a bit obscure, the important takeaway is this: until the underlying issue in the Erlang runtime is resolved, the path you’re inspecting and the path that’s actually being read from (or written to) may differ, so any application that build paths with user input should probably implement null-byte screening.


#15

Thank you, I couldn’t quite remember where I saw it. :slight_smile: It was also slightly referenced on this thread in regards to :only and :only_matching (which are also prefix based and therefore not vulnerable).


#16

from https://bugs.erlang.org/browse/ERL-370

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.

so you were mostly right but it is indeed a relatively minor issue :slight_smile:


#17

The 2nd issue (cookie de-serialization) is only minor issue since Elixir/Erlang does not mix data and code into objects.

Rails had a similar issue back in a day where it would de-serialize stuff from params/cookie (don’t remember exactly) into classes. Combine that with ability to re-define classes at run time and you have remote code execution at it’s finest.


#18

so did PHP https://www.owasp.org/index.php/PHP_Object_Injection


#19

RE the quoted erlang bug report text:

I don’t think @josevalim intends to say that the null byte issue with Plug is minor, just that this isn’t necessarily a security issue in OTP itself.


#20

that’s what I said, my opinion may or may not be the same as Jose’s :slight_smile:

To be precise about what I meant: the vulnerability might not be minor, but it is less severe than I thought (hence “relatively minor”).