From 18a0c923d0da4c8fb6e33b383dabd1d06bb22968 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Thu, 3 Aug 2023 13:08:37 -0400 Subject: [PATCH 1/8] Resolve information disclosure vulnerability through emoji pack archive download endpoint The pack name has been sanitized so an attacker cannot upload a media file called pack.json with their own handcrafted list of emoji files as arbitrary files on the filesystem and then call the emoji pack archive download endpoint with a pack name crafted to the location of the media file they uploaded which tricks Pleroma into generating a zip file of the target files the attacker wants to download. The attack only works if the Pleroma instance does not have the AnonymizeFilename upload filter enabled, which is currently the default. Reported by: graf@poast.org --- changelog.d/emoji-pack-sanitization.security | 1 + lib/pleroma/emoji/pack.ex | 1 + test/pleroma/emoji/pack_test.exs | 4 ++++ 3 files changed, 6 insertions(+) create mode 100644 changelog.d/emoji-pack-sanitization.security diff --git a/changelog.d/emoji-pack-sanitization.security b/changelog.d/emoji-pack-sanitization.security new file mode 100644 index 000000000..f3218abd4 --- /dev/null +++ b/changelog.d/emoji-pack-sanitization.security @@ -0,0 +1 @@ +Emoji pack loader sanitizes pack names diff --git a/lib/pleroma/emoji/pack.ex b/lib/pleroma/emoji/pack.ex index a361ea200..6e58f8898 100644 --- a/lib/pleroma/emoji/pack.ex +++ b/lib/pleroma/emoji/pack.ex @@ -285,6 +285,7 @@ defmodule Pleroma.Emoji.Pack do @spec load_pack(String.t()) :: {:ok, t()} | {:error, :file.posix()} def load_pack(name) do + name = Path.basename(name) pack_file = Path.join([emoji_path(), name, "pack.json"]) with {:ok, _} <- File.stat(pack_file), diff --git a/test/pleroma/emoji/pack_test.exs b/test/pleroma/emoji/pack_test.exs index 18b99da75..00001abfc 100644 --- a/test/pleroma/emoji/pack_test.exs +++ b/test/pleroma/emoji/pack_test.exs @@ -90,4 +90,8 @@ defmodule Pleroma.Emoji.PackTest do assert updated_pack.files_count == 1 end + + test "load_pack/1 ignores path traversal in a forged pack name", %{pack: pack} do + assert {:ok, ^pack} = Pack.load_pack("../../../../../dump_pack") + end end From 4befb3b1d02f32eb2c56f12e4684a7bb3167b0ee Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 00:46:52 +0200 Subject: [PATCH 2/8] Config: Restrict permissions of OTP config file --- lib/pleroma/config/release_runtime_provider.ex | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/lib/pleroma/config/release_runtime_provider.ex b/lib/pleroma/config/release_runtime_provider.ex index 91e5f1a54..9ec0f975e 100644 --- a/lib/pleroma/config/release_runtime_provider.ex +++ b/lib/pleroma/config/release_runtime_provider.ex @@ -20,6 +20,20 @@ defmodule Pleroma.Config.ReleaseRuntimeProvider do with_runtime_config = if File.exists?(config_path) do + # + %File.Stat{mode: mode} = File.lstat!(config_path) + + if Bitwise.band(mode, 0o007) > 0 do + raise "Configuration at #{config_path} has world-permissions, execute the following: chmod o= #{config_path}" + end + + if Bitwise.band(mode, 0o020) > 0 do + raise "Configuration at #{config_path} has group-wise write permissions, execute the following: chmod g-w #{config_path}" + end + + # Note: Elixir doesn't provides a getuid(2) + # so cannot forbid group-read only when config is owned by us + runtime_config = Config.Reader.read!(config_path) with_defaults From bd7381f2f4139c26d9dbb8aad77ce77be7777532 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 00:58:05 +0200 Subject: [PATCH 3/8] instance gen: Reduce permissions of pleroma directories and config files --- lib/mix/tasks/pleroma/instance.ex | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/mix/tasks/pleroma/instance.ex b/lib/mix/tasks/pleroma/instance.ex index 5c93f19ff..5d8b254a2 100644 --- a/lib/mix/tasks/pleroma/instance.ex +++ b/lib/mix/tasks/pleroma/instance.ex @@ -266,12 +266,20 @@ defmodule Mix.Tasks.Pleroma.Instance do config_dir = Path.dirname(config_path) psql_dir = Path.dirname(psql_path) + # Note: Distros requiring group read (0o750) on those directories should + # pre-create the directories. [config_dir, psql_dir, static_dir, uploads_dir] |> Enum.reject(&File.exists?/1) - |> Enum.map(&File.mkdir_p!/1) + |> Enum.each(fn dir -> + File.mkdir_p!(dir) + File.chmod!(dir, 0o700) + end) shell_info("Writing config to #{config_path}.") + # Sadly no fchmod(2) equivalent in Elixir… + File.touch!(config_path) + File.chmod!(config_path, 0o640) File.write(config_path, result_config) shell_info("Writing the postgres script to #{psql_path}.") File.write(psql_path, result_psql) @@ -290,8 +298,7 @@ defmodule Mix.Tasks.Pleroma.Instance do else shell_error( "The task would have overwritten the following files:\n" <> - (Enum.map(will_overwrite, &"- #{&1}\n") |> Enum.join("")) <> - "Rerun with `--force` to overwrite them." + Enum.map_join(will_overwrite, &"- #{&1}\n") <> "Rerun with `--force` to overwrite them." ) end end From 22df32b3f5cfe9fe0a4a97ff799df72c091b676e Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 22 Jun 2023 01:00:25 +0200 Subject: [PATCH 4/8] changelog: Entry for config permissions restrictions Closes: https://git.pleroma.social/pleroma/pleroma/-/issues/3135 --- changelog.d/otp_perms.security | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/otp_perms.security diff --git a/changelog.d/otp_perms.security b/changelog.d/otp_perms.security new file mode 100644 index 000000000..a3da1c677 --- /dev/null +++ b/changelog.d/otp_perms.security @@ -0,0 +1 @@ +- Reduced permissions of config files and directories, distros requiring greater permissions like group-read need to pre-create the directories \ No newline at end of file From 76e408e42d1da123a955b85490f05f6d810172f9 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 07:16:50 +0200 Subject: [PATCH 5/8] release_runtime_provider_test: chmod config for hardened permissions Git doesn't manages file permissions precisely enough for us. --- test/pleroma/config/release_runtime_provider_test.exs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/pleroma/config/release_runtime_provider_test.exs b/test/pleroma/config/release_runtime_provider_test.exs index 4e0d4c838..8ff578e63 100644 --- a/test/pleroma/config/release_runtime_provider_test.exs +++ b/test/pleroma/config/release_runtime_provider_test.exs @@ -17,6 +17,8 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "merged runtime config" do + assert :ok == File.chmod!("test/fixtures/config/temp.secret.exs", 0o640) + merged = ReleaseRuntimeProvider.load([], config_path: "test/fixtures/config/temp.secret.exs") @@ -25,6 +27,8 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "merged exported config" do + assert :ok == File.chmod!("test/fixtures/config/temp.exported_from_db.secret.exs", 0o640) + ExUnit.CaptureIO.capture_io(fn -> merged = ReleaseRuntimeProvider.load([], @@ -37,6 +41,9 @@ defmodule Pleroma.Config.ReleaseRuntimeProviderTest do end test "runtime config is merged with exported config" do + assert :ok == File.chmod!("test/fixtures/config/temp.secret.exs", 0o640) + assert :ok == File.chmod!("test/fixtures/config/temp.exported_from_db.secret.exs", 0o640) + merged = ReleaseRuntimeProvider.load([], config_path: "test/fixtures/config/temp.secret.exs", From c37561214a803f9011d5ec6af8b8c07e547c19ff Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 06:49:19 +0200 Subject: [PATCH 6/8] Force the use of amd64 runners for jobs using ci-base --- .gitlab-ci.yml | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 8b0381d11..91e568a32 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -32,7 +32,13 @@ before_script: after_script: - rm -rf _build/*/lib/pleroma +.using-ci-base: + tags: + - amd64 + build: + extends: + - .using-ci-base stage: build only: changes: &build_changes_policy @@ -44,6 +50,8 @@ build: - mix compile --force spec-build: + extends: + - .using-ci-base stage: test only: changes: @@ -57,6 +65,8 @@ spec-build: - mix pleroma.openapi_spec spec.json benchmark: + extends: + - .using-ci-base stage: benchmark when: manual variables: @@ -71,6 +81,8 @@ benchmark: - mix pleroma.load_testing unit-testing: + extends: + - .using-ci-base stage: test only: changes: *build_changes_policy @@ -94,6 +106,8 @@ unit-testing: path: coverage.xml unit-testing-erratic: + extends: + - .using-ci-base stage: test retry: 2 allow_failure: true @@ -129,6 +143,8 @@ unit-testing-erratic: # - mix test --trace --only federated unit-testing-rum: + extends: + - .using-ci-base stage: test only: changes: *build_changes_policy @@ -162,6 +178,8 @@ lint: - mix format --check-formatted analysis: + extends: + - .using-ci-base stage: test only: changes: *build_changes_policy From 5ac2b7417d052a493b38ee05b393ae7c78e89484 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 09:16:08 +0200 Subject: [PATCH 7/8] test: Fix warnings --- .../activity_pub/transmogrifier/emoji_react_handling_test.exs | 2 +- test/pleroma/web/mastodon_api/update_credentials_test.exs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/pleroma/web/activity_pub/transmogrifier/emoji_react_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/emoji_react_handling_test.exs index 9d99df27c..83bf59c6f 100644 --- a/test/pleroma/web/activity_pub/transmogrifier/emoji_react_handling_test.exs +++ b/test/pleroma/web/activity_pub/transmogrifier/emoji_react_handling_test.exs @@ -65,7 +65,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.EmojiReactHandlingTest do object = Object.get_by_ap_id(data["object"]) assert object.data["reaction_count"] == 1 - assert match?([[emoji, _]], object.data["reactions"]) + assert match?([[^emoji, _]], object.data["reactions"]) end test "it reject invalid emoji reactions" do diff --git a/test/pleroma/web/mastodon_api/update_credentials_test.exs b/test/pleroma/web/mastodon_api/update_credentials_test.exs index 57fa0f047..40f79d103 100644 --- a/test/pleroma/web/mastodon_api/update_credentials_test.exs +++ b/test/pleroma/web/mastodon_api/update_credentials_test.exs @@ -375,7 +375,7 @@ defmodule Pleroma.Web.MastodonAPI.UpdateCredentialsTest do "pleroma_background_image" => new_background_oversized }) - assert user_response = json_response_and_validate_schema(res, 413) + assert _user_response = json_response_and_validate_schema(res, 413) assert user.background == %{} clear_config([:instance, :upload_limit], upload_limit) From 57f74537486cf7f721679f125741de9008478b00 Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Fri, 4 Aug 2023 05:13:28 +0200 Subject: [PATCH 8/8] Release 2.5.3 --- CHANGELOG.md | 6 ++++++ mix.exs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f6fc6aaee..468ec1012 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed +## 2.5.3 + +### Security +- Emoji pack loader sanitizes pack names +- Reduced permissions of config files and directories, distros requiring greater permissions like group-read need to pre-create the directories + ## 2.5.2 ### Security diff --git a/mix.exs b/mix.exs index 79fd9c9ef..d1cdb151d 100644 --- a/mix.exs +++ b/mix.exs @@ -4,7 +4,7 @@ defmodule Pleroma.Mixfile do def project do [ app: :pleroma, - version: version("2.5.2"), + version: version("2.5.3"), elixir: "~> 1.11", elixirc_paths: elixirc_paths(Mix.env()), compilers: [:phoenix, :gettext] ++ Mix.compilers(),