Bug in signature validation renders Cipher unusable

[Fl4m3Ph03n1x]

Background

Our team was using cypher to create URL signatures in the client and validate them via a the Plug in the server.

Issue

However, even though we were using the test_mode flag set to true with the callback function to log errors, Cipher was still crashing and killing the server process.

Yes, the signatures were faulty, but the expected behaviour would be to Log the error instead of crashing the process.

RCA

After lengthy analysis I concluded that the bug is located in cipher/validate_plug.ex, namely in the following code snippet:

case validation do
      {:ok, _} -> conn

      {:error, error} ->
        # call user fun if given
        if opts[:error_callback], do: opts[:error_callback].(conn, error)

        if opts[:test_mode] do
          conn
        else
          conn
          |> send_resp(401, "unauthorized")
          |> halt
        end
end

The issue here is that this case clause is missing pattern matches, which causes it to throw an exception.

How could this be?

This is where things get tricky. Cipher is tightly coupled to a JSON parsing library, called Poison.

Cipher says in the mix.exs file that it supports several Poison versions. In fact it says it supports incompatible Poison versions.

{:poison, "~> 2.0 or ~> 3.0"}

These versions have incompatible APIs between them when reporting errors, so the code that pattern matches an error expression from Poison v2 does not match one from Poison v3. Not only that, but Poison v3 has known bugs in the error reporting API that cause errors to have different formats.

Poison v4 solved some of the issues, but the API breaks backwards compatibility.

And now comes the crux of the issue. Cipher's code and tests do not take into account all the different Poison versions, nor the specific bugs encountered in each version.

Possible solution

A possible solution would be to remake Cipher to use Poison v4, or Jason ( or preferably, to have a behaviour that would allow plugins that use different JSON parsers ) but that would take a tremendous effort that when taking in consideration other Cipher weaknesses ( such as the low security for the generated signatures ) would not be worth it.

We instead recommend you use an alternative to Cipher.

[rubencaro]

We instead recommend you use an alternative to Cipher.

We do. 😁