From fd63e66e3801bac7a3905b7c63edc697460653e7 Mon Sep 17 00:00:00 2001 From: michaeljguarino Date: Mon, 30 Sep 2024 23:51:37 -0400 Subject: [PATCH] Don't allow url based uploads This is technically somewhat insecure (not that bad, since requests are auto-ignored if not jpeg/png files). Should also think of a way to refactor the avatar upload out of the oidc login path entirely, and move it async, just not a priority. --- apps/core/lib/core/schema/account.ex | 2 +- apps/core/lib/core/schema/artifact.ex | 4 ++-- apps/core/lib/core/schema/crd.ex | 4 ++-- apps/core/lib/core/schema/file.ex | 2 +- apps/core/lib/core/schema/integration.ex | 4 ++-- apps/core/lib/core/schema/publisher.ex | 2 +- apps/core/lib/core/schema/repository.ex | 2 +- apps/core/lib/core/schema/terraform.ex | 4 ++-- apps/core/lib/core/schema/user.ex | 6 +++++- apps/core/lib/core/schema/version.ex | 2 +- apps/core/lib/core/services/users.ex | 1 + 11 files changed, 19 insertions(+), 14 deletions(-) diff --git a/apps/core/lib/core/schema/account.ex b/apps/core/lib/core/schema/account.ex index 0fd97ecc4..28e8235a8 100644 --- a/apps/core/lib/core/schema/account.ex +++ b/apps/core/lib/core/schema/account.ex @@ -63,7 +63,7 @@ defmodule Core.Schema.Account do |> unique_constraint(:name) |> validate_required([:name]) |> generate_uuid(:icon_id) - |> cast_attachments(attrs, [:icon], allow_urls: true) + |> cast_attachments(attrs, [:icon]) |> set_address_updated() |> reject_urls(:name) end diff --git a/apps/core/lib/core/schema/artifact.ex b/apps/core/lib/core/schema/artifact.ex index 39c3be334..825f2adcb 100644 --- a/apps/core/lib/core/schema/artifact.ex +++ b/apps/core/lib/core/schema/artifact.ex @@ -34,7 +34,7 @@ defmodule Core.Schema.Artifact do |> validate_required([:name, :platform, :type, :arch]) |> foreign_key_constraint(:repository_id) |> unique_constraint(:repository_id, name: index_name(:artifacts, [:repository_id, :name, :platform, :arch])) - |> cast_attachments(attrs, [:blob], allow_urls: true) + |> cast_attachments(attrs, [:blob]) |> add_sha(attrs) |> add_filesize(attrs) end @@ -52,4 +52,4 @@ defmodule Core.Schema.Artifact do end end def add_filesize(changeset, _), do: changeset -end \ No newline at end of file +end diff --git a/apps/core/lib/core/schema/crd.ex b/apps/core/lib/core/schema/crd.ex index fa81224c9..c246c6edc 100644 --- a/apps/core/lib/core/schema/crd.ex +++ b/apps/core/lib/core/schema/crd.ex @@ -21,7 +21,7 @@ defmodule Core.Schema.Crd do |> generate_uuid(:blob_id) |> foreign_key_constraint(:version_id) |> unique_constraint(:name, name: index_name(:crds, [:version_id, :name])) - |> cast_attachments(attrs, [:blob], allow_urls: true) + |> cast_attachments(attrs, [:blob]) |> validate_required([:name, :version_id]) end -end \ No newline at end of file +end diff --git a/apps/core/lib/core/schema/file.ex b/apps/core/lib/core/schema/file.ex index 425ca534d..8d4f6acbd 100644 --- a/apps/core/lib/core/schema/file.ex +++ b/apps/core/lib/core/schema/file.ex @@ -39,7 +39,7 @@ defmodule Core.Schema.File do model |> cast(attrs, @valid) |> generate_uuid(:blob_id) - |> cast_attachments(attrs, [:blob], allow_urls: true) + |> cast_attachments(attrs, [:blob]) |> foreign_key_constraint(:message_id) |> put_change(:filename, filename(upload)) |> put_change(:filesize, file_size(upload)) diff --git a/apps/core/lib/core/schema/integration.ex b/apps/core/lib/core/schema/integration.ex index 91a9dac78..747001c6c 100644 --- a/apps/core/lib/core/schema/integration.ex +++ b/apps/core/lib/core/schema/integration.ex @@ -51,7 +51,7 @@ defmodule Core.Schema.Integration do |> foreign_key_constraint(:repository_id) |> unique_constraint(:name, name: index_name(:integrations, [:repository_id, :name])) |> generate_uuid(:icon_id) - |> cast_attachments(attrs, [:icon], allow_urls: true) + |> cast_attachments(attrs, [:icon]) |> validate_required([:name, :spec]) end @@ -64,4 +64,4 @@ defmodule Core.Schema.Integration do end) end def validate(changeset, _), do: add_error(changeset, :spec, "No resource definition present for this repository") -end \ No newline at end of file +end diff --git a/apps/core/lib/core/schema/publisher.ex b/apps/core/lib/core/schema/publisher.ex index e9689a687..445566569 100644 --- a/apps/core/lib/core/schema/publisher.ex +++ b/apps/core/lib/core/schema/publisher.ex @@ -75,7 +75,7 @@ defmodule Core.Schema.Publisher do |> unique_constraint(:owner_id) |> validate_length(:name, max: 255) |> generate_uuid(:avatar_id) - |> cast_attachments(attrs, [:avatar], allow_urls: true) + |> cast_attachments(attrs, [:avatar]) end @stripe_valid ~w(billing_account_id)a diff --git a/apps/core/lib/core/schema/repository.ex b/apps/core/lib/core/schema/repository.ex index 4e96fd2ba..0787f7c0f 100644 --- a/apps/core/lib/core/schema/repository.ex +++ b/apps/core/lib/core/schema/repository.ex @@ -232,7 +232,7 @@ defmodule Core.Schema.Repository do |> unique_constraint(:name) |> validate_required([:name, :category]) |> generate_uuid(:icon_id) - |> cast_attachments(attrs, [:icon, :dark_icon, :docs], allow_urls: true) + |> cast_attachments(attrs, [:icon, :dark_icon, :docs]) end @keyvalid ~w(public_key private_key)a diff --git a/apps/core/lib/core/schema/terraform.ex b/apps/core/lib/core/schema/terraform.ex index ef9e0a5d4..5276fe861 100644 --- a/apps/core/lib/core/schema/terraform.ex +++ b/apps/core/lib/core/schema/terraform.ex @@ -49,7 +49,7 @@ defmodule Core.Schema.Terraform do |> unique_constraint(:name, name: index_name(:terraform, [:repository_id, :name])) |> foreign_key_constraint(:repository_id) |> bump_version(schema) - |> cast_attachments(attrs, [:package], allow_urls: true) + |> cast_attachments(attrs, [:package]) |> validate_required([:name, :repository_id]) end @@ -62,4 +62,4 @@ defmodule Core.Schema.Terraform do _ -> cs end end -end \ No newline at end of file +end diff --git a/apps/core/lib/core/schema/user.ex b/apps/core/lib/core/schema/user.ex index 2db594675..8bbb54e44 100644 --- a/apps/core/lib/core/schema/user.ex +++ b/apps/core/lib/core/schema/user.ex @@ -90,6 +90,10 @@ defmodule Core.Schema.User do timestamps() end + def mark_url_safe(), do: Process.put({__MODULE__, :url_safe}, true) + + def url_safe?(), do: !!Process.get({__MODULE__, :url_safe}) + def intercom_id(%__MODULE__{id: id}), do: Core.sha("#{id}:#{Core.conf(:intercom_salt)}") def service_account(query \\ __MODULE__, is_svc \\ :yes) @@ -209,7 +213,7 @@ defmodule Core.Schema.User do |> hash_password() |> generate_uuid(:avatar_id) |> change_markers(password_hash: :password_change) - |> cast_attachments(attrs, [:avatar], allow_urls: true) + |> cast_attachments(attrs, [:avatar], (if url_safe?(), do: [allow_urls: true], else: [])) |> set_email_changed() |> reject_passwordless() end diff --git a/apps/core/lib/core/schema/version.ex b/apps/core/lib/core/schema/version.ex index 2de9b4cd4..023890475 100644 --- a/apps/core/lib/core/schema/version.ex +++ b/apps/core/lib/core/schema/version.ex @@ -59,7 +59,7 @@ defmodule Core.Schema.Version do |> cast_assoc(:tags) |> validate_required([:version]) |> generate_uuid(:package_id) - |> cast_attachments(attrs, [:package], allow_urls: true) + |> cast_attachments(attrs, [:package]) |> unique_constraint(:chart_id, name: index_name(:versions, [:chart_id, :version])) |> unique_constraint(:terraform_id, name: index_name(:versions, [:terraform_id, :version])) |> foreign_key_constraint(:chart_id) diff --git a/apps/core/lib/core/services/users.ex b/apps/core/lib/core/services/users.ex index 6b00a5bfd..51470b463 100644 --- a/apps/core/lib/core/services/users.ex +++ b/apps/core/lib/core/services/users.ex @@ -367,6 +367,7 @@ defmodule Core.Services.Users do """ @spec bootstrap_user(Core.OAuth.method, map) :: user_resp def bootstrap_user(service, %{email: email} = attrs) do + User.mark_url_safe() case {service, get_user_by_email(email)} do {_, nil} -> attrs