Skip to content

Commit

Permalink
Improvements to Plug.Static and Plug.Crypto
Browse files Browse the repository at this point in the history
  • Loading branch information
José Valim committed Feb 28, 2017
1 parent c30ffae commit cc58306
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 5 deletions.
37 changes: 37 additions & 0 deletions lib/plug/crypto.ex
Expand Up @@ -5,6 +5,43 @@ defmodule Plug.Crypto do

use Bitwise

@doc """
A restricted version a `:erlang.binary_to_term/1` that
forbids possibly unsafe terms.
"""
def safe_binary_to_term(binary) when is_binary(binary) do
safe_terms(:erlang.binary_to_term(binary))
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

This comment has been minimized.

Copy link
@michalmuskala

michalmuskala Feb 28, 2017

Contributor

This excludes ports and funs. I'm sure funs are intentional, but are ports intentional as well?

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

@doc """
Masks the token on the left with the token on the right.
Expand Down
11 changes: 7 additions & 4 deletions lib/plug/session/cookie.ex
Expand Up @@ -121,12 +121,15 @@ defmodule Plug.Session.COOKIE do
binary
end

defp decode({:ok, binary}, :external_term_format, _log) do
defp decode({:ok, binary}, :external_term_format, log) do
{:term,
try do
:erlang.binary_to_term(binary)
Plug.Crypto.safe_binary_to_term(binary)
rescue
_ -> %{}
e ->
Logger.log log, "Plug.Session could not decode incoming session cookie. Reason: " <>
Exception.message(e)
%{}
end}
end

Expand All @@ -142,7 +145,7 @@ defmodule Plug.Session.COOKIE do
end

defp decode(:error, _serializer, log) do
Logger.log log, "Plug.Session could not decode incoming session cookie. " <>
Logger.log log, "Plug.Session could not verify incoming session cookie. " <>
"This may happen when the session settings change or a stale cookie is sent."
{nil, %{}}
end
Expand Down
2 changes: 1 addition & 1 deletion lib/plug/static.ex
Expand Up @@ -288,6 +288,6 @@ defmodule Plug.Static do
do: []

defp invalid_path?([h|_]) when h in [".", "..", ""], do: true
defp invalid_path?([h|t]), do: String.contains?(h, ["/", "\\", ":"]) or invalid_path?(t)
defp invalid_path?([h|t]), do: String.contains?(h, ["/", "\\", ":", "\0"]) or invalid_path?(t)
defp invalid_path?([]), do: false
end
9 changes: 9 additions & 0 deletions test/plug/crypto_test.exs
Expand Up @@ -26,4 +26,13 @@ defmodule Plug.CryptoTest do
refute masked_compare(<<1>>, <<>>, <<0>>)
refute masked_compare(<<0>>, <<1>>, <<0>>)
end

test "safe_binary_to_term" do
value = %{1 => {:foo, ["bar", 2.0, self(), make_ref()]}}
assert safe_binary_to_term(:erlang.term_to_binary(value)) == value

assert_raise ArgumentError, fn ->
safe_binary_to_term(:erlang.term_to_binary(%{1 => {:foo, [fn -> :bar end]}}))
end
end
end
12 changes: 12 additions & 0 deletions test/plug/static_test.exs
Expand Up @@ -167,6 +167,18 @@ defmodule Plug.StaticTest do
call(conn(:get, "/public/c:\\foo.txt"))
end
assert Plug.Exception.status(exception) == 400

exception = assert_raise Plug.Static.InvalidPathError,
"invalid path for static asset", fn ->
call(conn(:get, "/public/sample.txt%00.html"))
end
assert Plug.Exception.status(exception) == 400

exception = assert_raise Plug.Static.InvalidPathError,
"invalid path for static asset", fn ->
call(conn(:get, "/public/sample.txt\0.html"))
end
assert Plug.Exception.status(exception) == 400
end

test "returns 400 for invalid paths" do
Expand Down

0 comments on commit cc58306

Please sign in to comment.