I've forked Arc to Waffle and merged all open PRs

Wow… validating files uploads by extension, that’s really asking for trouble!

Glad you are fixing it, but this really leaves me concerned about the security approach taken on this library. Are their more unsafe approaches in the file upload process needing to be fixed?

2 Likes

You can call external tool to extract image size for you, like

identify -format '%wx%h' img.png 
# => 100x200

You can call it with System.cmd/3 .

4 Likes

Anyone for Waffles? An open-source file upload library for Elixir

4 Likes

Hey @achempion,

Please does Waffle support video uploads to aws s3 bucket, and can it generates links that can be used to play these videos on demand in a HTML page?

Thanks for your work.

2 Likes

It’s uploading library, you can upload any type of the file you want.
You can generate a link to the file as well.

4 Likes

I’ve published the new version v1.1.0. We’ve had 30 releases after the fork so far.

Now Waffle respects the content-disposition header for file uploads by url. It means it’ll save a file with a name from the header if it’s present.

Kudos to @ndarilek for the work.

7 Likes

Thanks for your work in Waffle, I’m newbie in Elixir, Using Arc I had this error

02:45:58.484 [warn] ExAws: HTTP ERROR: :nxdomain for URL: "https://s3.amazonaws.comuploads/company_logo/2e24af00-a7ad-4bd9-b5e2-8328f2690a9b_original.png?uploads=1" ATTEMPT: 10

I changed to use Waffle and I still get the same error.

This is really weird, because I tested in my local machine and it works. I’m using Phoenix, run the server in production mode and the image is uploaed to S3 successfully. But in production I have a E2 instance and I get the same error :nxdomain.. etc..

As you can see in the url the bucket name doesn’t appear, and also the host is joined with the uploads/company_logo directory.

I’m not sure if this is an issue of the ex_aws or is a issue that comes from Arc.

This is part of my code, if you can help me, I really appreciate, thanks!!

# The waffer_ecto module
defmodule Datasets.Storage.CompanyLogo do
  use Waffle.Definition
  use Waffle.Ecto.Definition

  @versions [:original, :thumb]
  @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

  def transform(:thumb, _) do
    {:convert, "-thumbnail 100x100^ -gravity center -extent 100x100 -format png", :png}
  end

  def filename(version, {_file, scope}) do
    "#{scope.uuid}_#{version}"
  end

  def storage_dir(_, {_, _}) do
    "uploads/company_logo"
  end
end

# config/prod.exs

# AWS Credentials
config :ex_aws,
  access_key_id: [{:system, "AWS_ACCESS_KEY_ID"}, :instance_role],
  secret_access_key: [{:system, "AWS_SECRET_ACCESS_KEY"}, :instance_role],
  json_codec: Jason

# Mailer using AWS
config :web, Mailer.Dispatcher, adapter: Bamboo.SesAdapter

#  Waffle config
config :waffle,
  storage: Waffle.Storage.S3,
  bucket: {:system, "S3_BUCKET"}

I just switched to Waffle at 1.1.0!
waffle_ecto was yelling at me saying it should be 1.0.0, though.
I marked it to override, it should be fine, yeah?

:wave:

Judging by the url (https://s3.amazonaws.comuploads) and your storage_dir function, maybe the string returned from storage_dir needs to have a leading / like

def storage_dir(_, {_, _}) do
  "/uploads/company_logo"
end
2 Likes

But not before you validate that is indeed a png image, otherwise you may open yourself to remote command execution in the case the supposed png file contains other stuff then just an image.

I wouldn’t expect identify to execute any code.

Even if you were sure it couldn’t, you couldn’t guarantee that a future version of identify wouldn’t be able to do so, due to a bug.

Lot’s of big security breaches happen by combining several minor overlooked things in a code base, therefore a developer should always strive to write defensive code, therefore I would never ask an external command to verify an image file that I am not sure to be really an image. But that’s me :wink:

I’m working on the file validation for the library right now. Hope I would finish it soon.

You can use System.cmd("file", ["--mime", "--brief", filepath]) call to get mime type for the file and include it inside the validation function.

We have the elixir-plug/mime library if you want to refer to extension directly instead of mime type of the file (you can use it to get mime types by extension).

This only maps mime type, doesn’t validate them, and using external tools from the OS to do validations always open my eyebrows… I really don’t like libraries that rely on it. But that’s me… maybe because I work in the security space.

What approach would you recommend to implement? Do you have any concerns about the file linux library?

As I said I just have concerns about using calls to the OS, because this will open new scopes of vulnerabilities, like path expansion to undesired locations in the event of a bug.

So if things can be checked from within the programming language without using OS tooling then I will go with it, but If is not really possible to implement it in Elixir, then you really need to resort to external tools.

I am not at all an expert in images manipulation and verification, but I am almost sure to have seen in the past binary matches being used to check for the file mime type, but don’t recall where it was.

EDIT

Found the library Group4Layers/ex_image_info

The code for png:

defmodule ExImageInfo.Types.PNG do


  @moduledoc false


  @behaviour ExImageInfo.Detector


  @mime "image/png"
  @ftype "PNG"


  @signature <<"PNG\r\n", 0x1A, "\n">>
  @signature_ihdr <<"IHDR">>


  ## Public API


  def seems?(<< _::bytes-size(1), @signature, _rest::binary >>), do: true
  def seems?(_), do: false


  def info(<< _::bytes-size(1), @signature, _::size(32), @signature_ihdr, width::size(32), height::size(32), _rest::binary >>), do: {@mime, width, height, @ftype}
  def info(_), do: nil


  def type(<< _::size(8), @signature, _::size(32), @signature_ihdr, _rest::binary >>), do: {@mime, @ftype}
  def type(_), do: nil


end

Not sure if this is the most secure way of validating them or not… As I said I am not an expert on the matter.

You can always integrate with an Erlang NIF to a C library that uses ImageMagick’s code (I believe in Linux it’s called libmagick?) or a Elixir<=>Rust bridge (namely rustler) to a Rust library that does [almost] the same. It’s more complex and you’re still at the mercy of native code’s capriciousness but it’s less brittle than calling external programs at least (although that can also be debatable depending on OS and library versions).

2 Likes

If your assumption is that the software that verifies that file format can be buggy, then there is no software that verifies the file format that would be surely bug free, therefore there is no way to verify the file format itself.

4 Likes

This is an external software, thus opens a new scope of vulnerabilities. As a developer you may not be familiar with all the ways hackers find to chain the most innocent approaches developers use for convenience when solving their problems, but anyone in the infosec community can tell you a lot of stories around chaining mistakes and assumptions made by developers. I am in the security space only for 2 years as a developer advocate, not as a security researcher, and I already seen enough to realise how our software industry is broken for how they approach software development… Bear in mind that I am not accusing anyone in specific here, it’s just the reality where we leave, and that unfortunately isn’t changing quickly enough, as per data-breaches growing month by month indicate.

I am aware of the issues by invoking a third party tool but what are the alternatives? Implementing your own solution in Erlang/Elixir may actually be less secure, as you can make mistakes and introduce bugs that have been addressed in more battle-tested solutions. Integrating a C-library brings other concerns. They all have trade-offs.

4 Likes