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).
This commit is contained in:
Ivan Tashkinov 2019-02-07 22:14:06 +03:00
parent d84392c9e0
commit 2c68cf7e9e
2 changed files with 9 additions and 9 deletions

View file

@ -26,12 +26,14 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
require Logger require Logger
@httpoison Application.get_env(:pleroma, :httpoison) @httpoison Application.get_env(:pleroma, :httpoison)
@local_mastodon_name "Mastodon-Local"
action_fallback(:errors) action_fallback(:errors)
def create_app(conn, params) do def create_app(conn, params) do
with cs <- App.register_changeset(%App{}, params) |> IO.inspect(), with cs <- App.register_changeset(%App{}, params),
{:ok, app} <- Repo.insert(cs) |> IO.inspect() do false <- cs.changes[:client_name] == @local_mastodon_name,
{:ok, app} <- Repo.insert(cs) do
res = %{ res = %{
id: app.id |> to_string, id: app.id |> to_string,
name: app.client_name, name: app.client_name,
@ -1154,16 +1156,13 @@ def login(conn, _) do
end end
defp get_or_make_app() do 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} {:ok, app}
else else
_e -> _e ->
cs = cs = App.register_changeset(%App{}, Map.put(find_attrs, :scopes, "read,write,follow"))
App.register_changeset(%App{}, %{
client_name: "Mastodon-Local",
redirect_uris: ".",
scopes: "read,write,follow"
})
Repo.insert(cs) Repo.insert(cs)
end end

View file

@ -37,6 +37,7 @@ def create_authorization(conn, %{
true <- Pbkdf2.checkpw(password, user.password_hash), true <- Pbkdf2.checkpw(password, user.password_hash),
{:auth_active, true} <- {:auth_active, User.auth_active?(user)}, {:auth_active, true} <- {:auth_active, User.auth_active?(user)},
%App{} = app <- Repo.get_by(App, client_id: client_id), %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 {:ok, auth} <- Authorization.create_authorization(app, user) do
# Special case: Local MastodonFE. # Special case: Local MastodonFE.
redirect_uri = redirect_uri =