Skip to content

Commit d416ce9

Browse files
authored
Fix: Harden file upload (#693)
1 parent 536d483 commit d416ce9

12 files changed

Lines changed: 232 additions & 27 deletions

File tree

lib/mindwendel_web/controllers/file_controller.ex

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,23 @@ defmodule MindwendelWeb.FileController do
1616
end
1717
end
1818

19+
@safe_mime_types %{
20+
"image/jpeg" => "image/jpeg",
21+
"image/png" => "image/png",
22+
"image/gif" => "image/gif",
23+
"application/pdf" => "application/pdf"
24+
}
25+
1926
defp send_attached_file(conn, attached_file_id) do
2027
attached_file = Attachments.get_attached_file(attached_file_id)
2128

2229
case StorageService.get_file(attached_file.path) do
2330
{:ok, decrypted_file} ->
24-
send_download(conn, {:binary, decrypted_file},
31+
conn
32+
|> put_resp_header("x-content-type-options", "nosniff")
33+
|> send_download({:binary, decrypted_file},
2534
filename: attached_file.name,
26-
content_type: attached_file.file_type,
35+
content_type: safe_content_type(attached_file.file_type),
2736
disposition: :inline
2837
)
2938

@@ -32,6 +41,10 @@ defmodule MindwendelWeb.FileController do
3241
end
3342
end
3443

44+
defp safe_content_type(file_type) do
45+
Map.get(@safe_mime_types, file_type, "application/octet-stream")
46+
end
47+
3548
defp render_404(conn) do
3649
conn
3750
|> put_status(:not_found)

lib/mindwendel_web/live/idea_live/form_component.ex

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,24 +129,52 @@ defmodule MindwendelWeb.IdeaLive.FormComponent do
129129
end
130130

131131
defp prepare_attachments(socket) do
132-
files =
133-
consume_uploaded_entries(socket, :attachment, fn %{path: path}, entry ->
134-
# The tmp uploaded file will be deleted directly after this function ends.
135-
# Copy it to a tmp folder first so the attachment changeset can process it.
136-
# See also this discussion https://github.com/elixir-waffle/waffle/issues/71
137-
filename = "#{entry.uuid}.#{mime_ext(entry.client_type)}"
138-
dest = "#{Path.dirname(path)}/#{filename}"
139-
File.cp!(path, dest)
140-
{:ok, %{path: dest, name: entry.client_name, file_type: entry.client_type}}
141-
end)
142-
143-
files
132+
consume_uploaded_entries(socket, :attachment, fn %{path: path}, entry ->
133+
# The tmp uploaded file will be deleted directly after this function ends.
134+
# Copy it to a tmp folder first so the attachment changeset can process it.
135+
# See also this discussion https://github.com/elixir-waffle/waffle/issues/71
136+
case detect_mime_type(path) do
137+
nil ->
138+
{:ok, :rejected}
139+
140+
file_type ->
141+
filename = "#{entry.uuid}.#{mime_ext(file_type)}"
142+
dest = "#{Path.dirname(path)}/#{filename}"
143+
File.cp!(path, dest)
144+
{:ok, %{path: dest, name: entry.client_name, file_type: file_type}}
145+
end
146+
end)
147+
|> Enum.reject(&(&1 == :rejected))
144148
end
145149

146150
defp remove_tmp_attachments(tmp_attachments) do
147151
Enum.each(tmp_attachments, fn tmp_attachment -> File.rm(tmp_attachment.path) end)
148152
end
149153

154+
@doc false
155+
def detect_mime_type(path) do
156+
case File.open(path, [:read, :binary]) do
157+
{:ok, fd} ->
158+
result =
159+
case IO.binread(fd, 4) do
160+
data when is_binary(data) -> match_magic_bytes(data)
161+
_ -> nil
162+
end
163+
164+
File.close(fd)
165+
result
166+
167+
_ ->
168+
nil
169+
end
170+
end
171+
172+
defp match_magic_bytes(<<0xFF, 0xD8, 0xFF, _>>), do: "image/jpeg"
173+
defp match_magic_bytes(<<0x89, 0x50, 0x4E, 0x47>>), do: "image/png"
174+
defp match_magic_bytes(<<0x47, 0x49, 0x46, 0x38>>), do: "image/gif"
175+
defp match_magic_bytes(<<0x25, 0x50, 0x44, 0x46>>), do: "application/pdf"
176+
defp match_magic_bytes(_), do: nil
177+
150178
defp mime_ext(client_type) do
151179
List.first(MIME.extensions(client_type))
152180
end

lib/mindwendel_web/live/lane_live/index_component.ex

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ defmodule MindwendelWeb.LaneLive.IndexComponent do
1212

1313
%{current_user: current_user, brainstorming: brainstorming} = socket.assigns
1414

15-
if lane && has_moderating_permission(brainstorming.id, current_user) do
15+
if lane && lane.brainstorming_id == brainstorming.id &&
16+
has_moderating_permission(brainstorming.id, current_user) do
1617
{:ok, _} = Lanes.delete_lane(lane)
1718
end
1819

priv/gettext/de/LC_MESSAGES/default.po

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,17 +475,17 @@ msgstr "Zusätzlicher Anhang"
475475
msgid "No filename"
476476
msgstr "Kein Dateiname"
477477

478-
#: lib/mindwendel_web/live/idea_live/form_component.ex:156
478+
#: lib/mindwendel_web/live/idea_live/form_component.ex:184
479479
#, elixir-autogen, elixir-format
480480
msgid "File type is not allowed"
481481
msgstr "Dateityp nicht erlaubt"
482482

483-
#: lib/mindwendel_web/live/idea_live/form_component.ex:154
483+
#: lib/mindwendel_web/live/idea_live/form_component.ex:182
484484
#, elixir-autogen, elixir-format
485485
msgid "The selected file is too large"
486486
msgstr "Datei ist zu groß"
487487

488-
#: lib/mindwendel_web/live/idea_live/form_component.ex:155
488+
#: lib/mindwendel_web/live/idea_live/form_component.ex:183
489489
#, elixir-autogen, elixir-format
490490
msgid "Too many files selected"
491491
msgstr "Zu viele Dateien ausgewählt"

priv/gettext/default.pot

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -474,17 +474,17 @@ msgstr ""
474474
msgid "No filename"
475475
msgstr ""
476476

477-
#: lib/mindwendel_web/live/idea_live/form_component.ex:156
477+
#: lib/mindwendel_web/live/idea_live/form_component.ex:184
478478
#, elixir-autogen, elixir-format
479479
msgid "File type is not allowed"
480480
msgstr ""
481481

482-
#: lib/mindwendel_web/live/idea_live/form_component.ex:154
482+
#: lib/mindwendel_web/live/idea_live/form_component.ex:182
483483
#, elixir-autogen, elixir-format
484484
msgid "The selected file is too large"
485485
msgstr ""
486486

487-
#: lib/mindwendel_web/live/idea_live/form_component.ex:155
487+
#: lib/mindwendel_web/live/idea_live/form_component.ex:183
488488
#, elixir-autogen, elixir-format
489489
msgid "Too many files selected"
490490
msgstr ""

priv/gettext/en/LC_MESSAGES/default.po

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,17 +475,17 @@ msgstr ""
475475
msgid "No filename"
476476
msgstr ""
477477

478-
#: lib/mindwendel_web/live/idea_live/form_component.ex:156
478+
#: lib/mindwendel_web/live/idea_live/form_component.ex:184
479479
#, elixir-autogen, elixir-format
480480
msgid "File type is not allowed"
481481
msgstr ""
482482

483-
#: lib/mindwendel_web/live/idea_live/form_component.ex:154
483+
#: lib/mindwendel_web/live/idea_live/form_component.ex:182
484484
#, elixir-autogen, elixir-format
485485
msgid "The selected file is too large"
486486
msgstr ""
487487

488-
#: lib/mindwendel_web/live/idea_live/form_component.ex:155
488+
#: lib/mindwendel_web/live/idea_live/form_component.ex:183
489489
#, elixir-autogen, elixir-format
490490
msgid "Too many files selected"
491491
msgstr ""

priv/gettext/it/LC_MESSAGES/default.po

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -475,17 +475,17 @@ msgstr "Allegato aggiuntivo"
475475
msgid "No filename"
476476
msgstr "Nessun nome file"
477477

478-
#: lib/mindwendel_web/live/idea_live/form_component.ex:156
478+
#: lib/mindwendel_web/live/idea_live/form_component.ex:184
479479
#, elixir-autogen, elixir-format
480480
msgid "File type is not allowed"
481481
msgstr "Il tipo di file non è consentito"
482482

483-
#: lib/mindwendel_web/live/idea_live/form_component.ex:154
483+
#: lib/mindwendel_web/live/idea_live/form_component.ex:182
484484
#, elixir-autogen, elixir-format
485485
msgid "The selected file is too large"
486486
msgstr "Il file selezionato è troppo grande"
487487

488-
#: lib/mindwendel_web/live/idea_live/form_component.ex:155
488+
#: lib/mindwendel_web/live/idea_live/form_component.ex:183
489489
#, elixir-autogen, elixir-format
490490
msgid "Too many files selected"
491491
msgstr "Sono stati selezionati troppi file"

test/fixtures/test_image.png

1.19 KB
Loading

test/fixtures/test_malicious.svg

Lines changed: 5 additions & 0 deletions
Loading

test/mindwendel_web/controllers/file_controller_test.exs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,43 @@ defmodule MindwendelWeb.FileControllerTest do
2424

2525
assert get(conn, ~p"/files/#{file.id}").resp_body == "test"
2626
end
27+
28+
test "sets x-content-type-options nosniff header", %{conn: conn} do
29+
file =
30+
Factory.insert!(:file,
31+
path: "/uploads/encrypted-file-controller-test.jpg",
32+
name: "test.jpg",
33+
file_type: "image/jpeg"
34+
)
35+
36+
response = get(conn, ~p"/files/#{file.id}")
37+
assert get_resp_header(response, "x-content-type-options") == ["nosniff"]
38+
end
39+
40+
test "serves allowed mime type as-is", %{conn: conn} do
41+
file =
42+
Factory.insert!(:file,
43+
path: "/uploads/encrypted-file-controller-test.jpg",
44+
name: "test.jpg",
45+
file_type: "image/jpeg"
46+
)
47+
48+
response = get(conn, ~p"/files/#{file.id}")
49+
[content_type] = get_resp_header(response, "content-type")
50+
assert content_type =~ "image/jpeg"
51+
end
52+
53+
test "serves unknown mime type as application/octet-stream", %{conn: conn} do
54+
file =
55+
Factory.insert!(:file,
56+
path: "/uploads/encrypted-file-controller-test.jpg",
57+
name: "test.svg",
58+
file_type: "image/svg+xml"
59+
)
60+
61+
response = get(conn, ~p"/files/#{file.id}")
62+
[content_type] = get_resp_header(response, "content-type")
63+
assert content_type =~ "application/octet-stream"
64+
end
2765
end
2866
end

0 commit comments

Comments
 (0)