Potential RCE via :erlang.binary_to_term

[vovayartsev]

This is explained well in https://paraxial.io/blog/elixir-rce

Basically, the code below will print I was executed!!!

f = fn _, _ -> IO.puts "I was executed!!!"; {:halt, []} end
serialized = :erlang.term_to_binary(f)
serialized |> :erlang.binary_to_term() |> Enum.to_list()

In Nebulex context this means that any iteration over structures fetched from Nebulex creates an RCE vulnerability through Redis.
Yes, normally Redis is not writable from the outside.... But still it's a "security weakness".

One potential fix would be using https://hexdocs.pm/plug_crypto/Plug.Crypto.html#non_executable_binary_to_term/2

The downsides are:

• dependency on another library plug_crypto (a small one and with no other runtime deps)
• performance implications (because non_executable_binary_to_term would iterate over each result fetched from the cache)
• backward in-compatibility (what if someone indeed wanted to store anonymous functions in the cache 🤷)

@cabol I'm happy to make a PR if you suggest a resolution for the above concerns 🙏

[cabol]

Hi @vovayartsev !!

First of all, thanks for bringing this up. I'm glad to discuss it. Let me share my thoughts.

I agree with what you outlined above. However, I don't think this is an issue that should be addressed by Nebulex, IMO, it is more on the Nebulex client/user side. Nebulex is just the Cache wrapper, that can be seen as the cache itself in this scenario. This means what is stored in the cache is the user's responsibility. Like you said, what if a user wants to store an anonymous function? From the Nebulex standpoint, we shouldn't prevent that. The validation of the data to be stored (if you want any) should come from the client/user logic. Besides, the NebulexRedisAdapter is not iterating over the fetched data (or evaluating the fetched value(s)). For example, if you do this:

iex> f = fn _, _ -> IO.puts "I was executed!!!"; {:halt, []} end
iex> MyApp.Cache.put("fun", f)
:ok

iex> value = MyApp.Cache.get("fun") # Nebulex just returns what it was stored
iex> is_function(value)
true

So, Nebulex is not evaluating the fetched values or iterating over them, it just returns the stored data. Therefore, the issue itself is on the business logic using the adapter (on the user side), since it is where the value may be potentially evaluated. Does it make sense?

Now, let's say you want to add extra security validating the values to be stored in the cache. You can do that by providing your own serializer via the :serializer option (see NebulexRedisAdapter.Serializer for more info). In that way, you could validate the values before inserting them into the cache.

Anyway, I just wanted to share my thoughts about this, but let me know what you think.

[vovayartsev]

Thank for the very detailed reply @cabol !

Providing an own serializer via the :serializer option sounds like the most flexible approach indeed and it works great for me personally.

As for community (vs just me) - what do you think is the best way to raise awareness?
I could think of providing a reference implementation of the custom serializer in the docs,
or accepting a documented serializer_opts: [decode_value: [safe: true]] option which would depend on https://hexdocs.pm/plug_crypto when the option is set.

[cabol]

Thank would be great. I can think of a couple of options:

• As you suggest, we could add a guide (guides/custom-serializer.md) explaining how to implement a custom serializer "providing a reference implementation of the custom serializer in the docs, or accepting a documented serializer_opts: [decode_value: [safe: true]] option which would depend on https://hexdocs.pm/plug_crypto when the option is set."
• You could also create a separate repo with this custom serializer and then we can add it to the adapter's doc as an option. I like this option because it will be available for people who want to use this approach.

[vovayartsev]

You could also create a separate repo with this custom serializer

Challenge accepted 😎