From 2c68cf7e9ee6718f83f2209e6b009b02b50bc8f4 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 7 Feb 2019 22:14:06 +0300 Subject: [PATCH] OAuth2 security fixes: redirect URI validation, "Mastodon-Local" security breach fix. (`POST /api/v1/apps` could create "Mastodon-Local" app wth any redirect_uris, and if that happened before /web/login is accessed for the first time then Pleroma used this externally created record with arbitrary redirect_uris and client_secret known by creator). --- .../web/mastodon_api/mastodon_api_controller.ex | 17 ++++++++--------- lib/pleroma/web/oauth/oauth_controller.ex | 1 + 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index b6a3c895c..dbe7c2554 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -26,12 +26,14 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do require Logger @httpoison Application.get_env(:pleroma, :httpoison) + @local_mastodon_name "Mastodon-Local" action_fallback(:errors) def create_app(conn, params) do - with cs <- App.register_changeset(%App{}, params) |> IO.inspect(), - {:ok, app} <- Repo.insert(cs) |> IO.inspect() do + with cs <- App.register_changeset(%App{}, params), + false <- cs.changes[:client_name] == @local_mastodon_name, + {:ok, app} <- Repo.insert(cs) do res = %{ id: app.id |> to_string, name: app.client_name, @@ -1154,16 +1156,13 @@ def login(conn, _) do end defp get_or_make_app() do - with %App{} = app <- Repo.get_by(App, client_name: "Mastodon-Local") do + find_attrs = %{client_name: @local_mastodon_name, redirect_uris: "."} + + with %App{} = app <- Repo.get_by(App, find_attrs) do {:ok, app} else _e -> - cs = - App.register_changeset(%App{}, %{ - client_name: "Mastodon-Local", - redirect_uris: ".", - scopes: "read,write,follow" - }) + cs = App.register_changeset(%App{}, Map.put(find_attrs, :scopes, "read,write,follow")) Repo.insert(cs) end diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index 4d4e85836..8ec963c79 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -37,6 +37,7 @@ def create_authorization(conn, %{ true <- Pbkdf2.checkpw(password, user.password_hash), {:auth_active, true} <- {:auth_active, User.auth_active?(user)}, %App{} = app <- Repo.get_by(App, client_id: client_id), + true <- redirect_uri in String.split(app.redirect_uris), {:ok, auth} <- Authorization.create_authorization(app, user) do # Special case: Local MastodonFE. redirect_uri =