From 5bfb6830c9724cca719095b368aa9bf714098cb9 Mon Sep 17 00:00:00 2001 From: Sam Saffron Date: Wed, 27 May 2020 10:58:22 +1000 Subject: [PATCH] SECURITY: missing security check prior to redirect In some rare cases, if a user knows the exact title of a topic they could possibly determine that it really exists in the system --- app/controllers/topics_controller.rb | 2 ++ spec/requests/topics_controller_spec.rb | 11 +++++++++++ 2 files changed, 13 insertions(+) diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 211e94ce8d..bca0bde2de 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -945,6 +945,8 @@ class TopicsController < ApplicationController end def redirect_to_correct_topic(topic, post_number = nil) + guardian.ensure_can_see!(topic) + url = topic.relative_url url << "/#{post_number}" if post_number.to_i > 0 url << ".json" if request.format.json? diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 9649f57f5b..bc80fa9283 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1357,6 +1357,17 @@ RSpec.describe TopicsController do expect(response).to redirect_to(topic.relative_url) end + it 'will return a 403 if you try to redirect to a topic you have no access to' do + category = Fabricate(:category) + category.set_permissions(Group::AUTO_GROUPS[:staff] => :full) + category.save! + + topic.update!(category_id: category.id) + get "/t/#{topic.slug}" + + expect(response.status).to eq(403) + end + it 'can find a topic when a slug has a number in front' do another_topic = Fabricate(:post).topic