Perhaps the “blind” here has meaning I haven’t heard before but SQL injection is not possible with the Ecto querying API. Ecto always uses parameterized queries, so values never end up in the SQL string at all.

You do have an issue with |> String.to_atom() since this lets a hostile API user use arbitrary amounts of memory, so that’s not great, but I’m not clear on how there’s an SQL issue.

EDIT: After reading What Are Blind SQL Injections | Acunetix it doesn’t look like you have to worry about this at all. These kinds of attacks are only possible if you interpolate user values into the SQL string, which Ecto never does.

You might want to use String.to_existing_atom/1 or whitelist what params["target"] can be.

Thanks @idi527 and @benwilson512.

I dont use Ecto query yet, this is a different topic but the learning curve was very steep to me at the time (i’ve created some extra fun inside Repo and use a lot of Postgres JSONB functions).

So i’ve improved overall security by enhancing those parts:

config.exs

Added HTTP Status code for NGINX 444.

  config :plug,
    :statuses, %{
      444 => "CONNECTION CLOSED WITHOUT RESPONSE"
    }

checked allowed params

  def allowed(allowed_params, params) do
    allowed_params
    |> Enum.map(fn x ->
      {x, Map.get(params, x)}
    end)
    |> Enum.filter(fn {_x, y} ->
      y != nil
    end)
    |> Enum.into(%{})
  end

check allowed params values

  def params_values(params) when is_map(params) do
    regex =
      ~r/^[\w\d\/,_]*$/
    params
    |> Enum.map(fn {_x, y} ->
      regex
      |> Regex.match?(y)
    end)
    |> Enum.member?(false)
    |> Kernel.not()
  end

try execute query

try do
  items =
    apply(KPI, target, [subdomain, params])
  API.render_items(conn, items)
rescue
  _ ->
    API.render_halt(conn)
end

Render 444 response

  def render_halt(conn) do
    conn
    |> Conn.put_status(444)
    |> render_error(nil)
  end

  def render_error(conn, status) do
    Changes.api_audit(conn, status)
    conn
    |> put_view(MyApp.ErrorsView)
    |> render("error.json", status: status)
    |> halt()
  end

I confess I’m a little confused about what you’re concerned about here. You’re talking about SQL attacks but you aren’t showing any SQL you’re showing HTTP parameter validation which, while useful, is the LEAST reliable way to protect yourself from SQL attacks. If you aren’t using ecto, how are you running SQL queries?

Also you still have a memory leak here that a hostile party could use to make your system use arbitrary amounts of memory:

    target =
      params["target"]
      |> String.to_atom()

I don’t use Ecto Syntax for selects, straight SQL queries like this one:

query =
	"SELECT
	...
	FROM #{table1} AS t1
	...
	WHERE
	#{from}
	...
	#{AppCommon.filter_events("t1", "t3", "t4")}
	#{filter_table}
	#{order}
	#{limit}"
_results =
	query
	|> Repo.execute_all([]) 

def execute(sql, params) do
	SQL.query!(__MODULE__, sql, params)
end

Also you still have a memory leak here that a hostile party could use to make your system use arbitrary amounts of memory:

    target =
      params["target"]
      |> String.to_atom()

Only allowed params are converted to atoms:

  def kpi_allowed_target(params) do
    allowed_targets =
      [
        ...
      ]
    target =
      params["target"]
    allowed_targets
    |> Enum.member?(target)
    |> case do
      true ->
        target
        |> String.to_atom()
      false ->
        nil
    end
  end

As a minor note the execute function you have there seems the equivalent of the built in query! function. IE you can just do Repo.query!(sql, parameters).

What you have is either entirely fine or super dangerous depending on where the interpolation is coming from. If you make sure to NEVER interpolate user data then you’re safe. If you interpolate any user data you’re vulnerable to attack.

This is why the Ecto query DSL is nice, it can make sure at compile time that it’s impossible to interpolate user values.

@benwilson512 - could you unpack it a little more?

Atoms are Not GC’d, and therefore the BEAM puts a limit on how many are allowed to exist. This limit is 2**20 by default.

Therefore using String.to_atom/1 at runtime is considered a flaw.

Instead String.to_existing_atom/1 is preferred or even better an explicit conversion function built from a whitelist.

I understand that in the original context, that would theoretically allow third party to “bombard” the endpoint with random strings until the limit is reached, right? What happens then? Does the machine handle it somehow “gracefully”, or just quits with a kind of “LimitReachedError”?

The BEAM will do a hard exit.

Everything will crash. There is no chance to recover or to gracefully shut down anything.

From the view of your application it’s not different from a kill -9.