OAuth2 security fixes: redirect URI validation, "Mastodon-Local" security breach...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Thu, 7 Feb 2019 19:14:06 +0000 (22:14 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Thu, 7 Feb 2019 19:14:06 +0000 (22:14 +0300)
(`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).

lib/pleroma/web/mastodon_api/mastodon_api_controller.ex
lib/pleroma/web/oauth/oauth_controller.ex

index b6a3c895c803f087c7b9fcaffd2e50979e546981..dbe7c25542b061601211d8ae3c50d246ddc9510a 100644 (file)
@@ -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 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController 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
index 4d4e858361e5859b6b471e684c65bc8848d07889..8ec963c79f557e2000e49e21ed1f8511dd3c891e 100644 (file)
@@ -37,6 +37,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
          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 =