Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use magic bytes for image validations instead of extension #14

Open
achempion opened this issue Sep 9, 2019 · 1 comment · May be fixed by #69
Open

Use magic bytes for image validations instead of extension #14

achempion opened this issue Sep 9, 2019 · 1 comment · May be fixed by #69
Assignees

Comments

@achempion
Copy link
Member

to address the stavro/arc#263 from @speeddragon and stavro/arc#73 from @cichaczem

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.

@achempion achempion self-assigned this Sep 9, 2019
@achempion
Copy link
Member Author

achempion commented Sep 9, 2019

I agree that it should be a part of the project and we should at least provide an example how to properly validate the files. I'm looking into how to implement this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant