CVE-2022-1810: Authorization Bypass Through User-Controlled Key (CWE-639)
value: Authorization Bypass Through User-Controlled Key in GitHub repository publify/publify prior to 9.2.9.
CWE description: The system's authorization functionality does not prevent one user from gaining access to another user's data or record by modifying the key value identifying the data.
CVSS v3 base score: 4.3
Published: 2022-05-23T12:16Z
Repository: https://github.com/publify/publify
Language: Ruby
Files changed: publify_core/app/controllers/admin/content_controller.rb, publify_core/spec/controllers/admin/content_controller_spec.rb
Commit date: 2022-05-22 13:24:24 +0200
Commit message: Use only the main id parameter to find Article to update

Diff: diff --git a/publify_core/app/controllers/admin/content_controller.rb b/publify_core/app/controllers/admin/content_controller.rb
index a3b487d415..bab8e1d38a 100644
--- a/publify_core/app/controllers/admin/content_controller.rb
+++ b/publify_core/app/controllers/admin/content_controller.rb
@@ -56,13 +56,13 @@ class Admin::ContentController < Admin::BaseController
       render "new", layout: "editor"
     end
   end
 
   def update
-    return unless access_granted?(params[:id])
+    id = params[:id]
+    return unless access_granted?(id)
 
-    id = params[:article][:id] || params[:id]
     @article = Article.find(id)
 
     if params[:article][:draft]
       fetch_fresh_or_existing_draft_for_article
     else
diff --git a/publify_core/spec/controllers/admin/content_controller_spec.rb b/publify_core/spec/controllers/admin/content_controller_spec.rb
index 27e8168042..ec4dff0a22 100644
--- a/publify_core/spec/controllers/admin/content_controller_spec.rb
+++ b/publify_core/spec/controllers/admin/content_controller_spec.rb
@@ -525,10 +525,30 @@ describe Admin::ContentController, type: :controller do
 
         it { expect(response).to redirect_to(action: "index") }
         it { expect(article.reload.text_filter.name).to eq("textile") }
         it { expect(article.reload.body).to eq(body) }
       end
+
+      context "with an owned article and another user's article" do
+        let(:article) { create(:article, body: "another *textile* test", user: publisher) }
+        let(:other_article) { create(:article, body: "other article") }
+        let(:body) { "not the *same* text" }
+
+        before do
+          put :update,
+              params: { id: article.id,
+                        article: { id: other_article.id, body: body } }
+        end
+
+        it "ignores the extra id passed in the article parameters" do
+          aggregate_failures do
+            expect(response).to redirect_to(action: "index")
+            expect(article.reload.body).to eq(body)
+            expect(other_article.reload.body).not_to eq(body)
+          end
+        end
+      end
     end
   end
 
   describe "#auto_complete_for_article_keywords" do
     let(:publisher) { create(:user, :as_publisher) }

Vulnerable code:     return unless access_granted?(params[:id])
    id = params[:article][:id] || params[:id]
Fixed code:     id = params[:id]
    return unless access_granted?(id)

      context "with an owned article and another user's article" do
        let(:article) { create(:article, body: "another *textile* test", user: publisher) }
        let(:other_article) { create(:article, body: "other article") }
        let(:body) { "not the *same* text" }

        before do
          put :update,
              params: { id: article.id,
                        article: { id: other_article.id, body: body } }
        end

        it "ignores the extra id passed in the article parameters" do
          aggregate_failures do
            expect(response).to redirect_to(action: "index")
            expect(article.reload.body).to eq(body)
            expect(other_article.reload.body).not_to eq(body)
          end
        end
      end