Plug vulnerabilities: impact assessment

Posted 2017-03-01 13:16:28.000000

Two vulnerabilities were recently found and fixed in Plug. This being the 4th most popular package on hex.pm, as well as a fundamental part of Phoenix, any vulnerability in Plug is going to be high profile. But I think we can safely say that most users are not going to be impacted.

Please note that these are my own personal opinions, based solely on information publicly available at the time of writing. With that out of the way, let’s have a look.

Null byte injection in Plug.Static

Until yesterday’s fix, Plug.Static did not consider a null byte in a path to be invalid. Inside Elixir, a null byte in a string is just a weird character, but to the operating system it signifies the end of the string, and it is (likely) going to ignore any characters that follow:

iex(1)> File.read!("mix.exs\0some nonsense here")
"defmodule MyApp.Mixfile do\n  use Mix.Project\n\n  [...]"

An attacker might use this to bypass some validations the Elixir app would do prior to handing the path over to Plug.Static. For instance, if the application uses an ACL to control which files a user has access to, maybe based on who uploaded it, an attacker may be able to retrieve a file from another user.

Imagine Alice is allowed to access /documents/sales_forecast.xls and Carol is not, Carol could request /documents/sales_forecast.xls%00gotcha instead. For Elixir, this is a different path, but for the OS it is not. Assuming the application allows Carol to access this (weird) path, for example because Carol previously uploaded a file by that name, the ACL won’t stop Carol, but the OS will actually return the contents of Alice’s file.

Please note that, while the issue is more likely to affect sites that support user-uploaded content, it is not a precondition.

Update (March 7):

It’s been a week since the fixes have been released, and José has said he will provide more details today about the impact of the vulnerabilities. I expect the core team will use the following example attack to illustrate how the Plug.Static vulnerability can be used for XSS (cross-site scripting):

The attacker creates a user account with the vulnerable site, and uploads an HTML page with embedded JS code as their profile picture. The server stores the file as attacker.png, and serves it through Plug.Static, normally when referenced from an image tag as “http://example.net/profile_pics/attacker.png”.

The attacker now tricks a user into opening “http://example.net/profile_pics/attacker.png%00/index.html”, for example through a phishing email. Since Plug.Static sees a .html extension it serves up the file with “Content-Type: text/html”, but Erlang ignores everything after the zero byte and returns the contents of the crafted profile pic. The browser loads the page and executes the JS in it, in the context of the target site domain. This gives the attacker access to privileged information, such as cookies or local storage for the compromised site.

This is definitely a plausible scenario, probably more so than the ACL example I gave above. If you use the profile image pattern described above as part of your application, or something similar, you should definitely upgrade (you should anyway). And please note that the underlying issue can bite you even if you don’t use Plug.Static.

Code injecting in session cookie

When using the Plug.Session.COOKIE session store, the session key/value store is serialized and deserialized using :erlang.term_to_binary/1 and :erlang.binary_to_term/1. The deserialization only takes place once the integrity of the cookie value has been verified, to make sure it hasn’t been tampered with by a malicious client. Because in Elixir/Erlang a function is a term (almost) like any other, an attacker who can pass the integrity check could submit code that might get executed on the server.

Of course, if an attacker manages to get hold of the application’s signing key (typically stored in config/prod.secret.exs), your application is basically compromised: the attacker can prepare their own session state, serialize and sign it, and submit it in a cookie. This would allow the attacker to sign in as an arbitrary user, or even an admin, without knowing the password. It goes without saying that you should protect the secret_key_base value for your production servers, and renew it if there is any reason to believe it was compromised.

The issue with code injection is that it can be used to further escalate the attack: for instance, the attacker may be able to trigger arbitrary DB statement or OS shell commands. However, serializing a function into the session store is not sufficient to get that code executed. Only if the application explicitly invokes a value retrieved from the session store does it give the attacker the vector they need:

f = Plug.Conn.get_session(conn, :my_session_function)
f.("call the function")

Storing functions in the session store is just… weird. It should be considered code smell even before looking at the security implications.

Update (March 19):

Griffin Byatt, the reporter of the vulnerabilities, has published more details on a potential attack vector for the code injection issue. He points out that functions implement the Enumerable protocol, which allows an attacker to get the injected function triggered even if the developer only ever intended to store Enumerable data structures (e.g. lists) in the session.

In general I would say you should not use the built-in term_to_binary and binary_to_term functions for serialization of data that crosses trust boundaries. Instead, use an industry standard serialization format, or a less powerful version of binary_to_term as Plug now does.

Conclusion

It’s good to see the Elixir core team jump into action and fix potential issues with a library. For most users this may be false alarm, but if nothing else this was a good exercise in spreading awareness and getting people to check what it would take to perform a critical upgrade if and when a serious one does come along.

Still, I would encourage everyone to upgrade, not just as an exercise, but also because it is entirely possible that I may have missed something :-)

Hat tip to @micmus for his feedback.