stavro / arc

:paperclip: Flexible file upload and attachment library for Elixir
1.16k stars 210 forks source link

Use magic bytes for image validations instead of extension #263

Open speeddragon opened 6 years ago

speeddragon commented 6 years ago

Since last vulnerabilities with GhostScript, and because ImageMagick use it (for example when we use convert), should we include and modify the current examples to use ones that check for magic bytes ?

So instead of using this for image validation,

defmodule Avatar do
  use Arc.Definition
  @extension_whitelist ~w(.jpg .jpeg .gif .png)

  def validate({file, _}) do   
    file_extension = file.file_name |> Path.extname() |> String.downcase()
    Enum.member?(@extension_whitelist, file_extension)
  end
end
defmodule Helper do
@doc """
  JPG magic bytes: 0xffd8
  """
  @spec is_jpg(String.t()) :: boolean
  def is_jpg(file) do
    with {:ok, file_content} <- :file.open(file, [:read, :binary]),
         {:ok, <<255, 216>>} <- :file.read(file_content, 2) do
      true
    else
      _error ->
        false
    end
  end

  @doc """
  PNG magic bytes: 0x89504e470d0a1a0a
  """
  @spec is_png(String.t()) :: boolean
  def is_png(file) do
    with {:ok, file_content} <- :file.open(file, [:read, :binary]),
         {:ok, <<137, 80, 78, 71, 13, 10, 26, 10>>} <- :file.read(file_content, 8) do
      true
    else
      _error ->
        false
    end
  end
end

defmodule Avatar do
  use Arc.Definition
  @extension_whitelist ~w(.jpg .jpeg .gif .png)

  def validate({file, _}) do   
    file_extension = file.file_name |> Path.extname() |> String.downcase()
    Enum.member?(@extension_whitelist, file_extension) && (Helper.is_jpg(file.path) || Helper.is_png(file.path))
  end
end

I can also try to add for GIF or other file formats.

Betree commented 5 years ago

I think this should be part of Arc too. If we provide functions to modify images, we should also provide the tools to verify them.

This can have a security impact on numerous projects, so definitely +1

stephenmoloney commented 5 years ago

If I follow this correctly, Fastimage can do this, specifically Fastimage.type/2 (https://hexdocs.pm/fastimage/1.0.0-rc4/Fastimage.html#type/2)