From 3a73f29928c86dd61b4723f4eb8fd51b7df85fe6 Mon Sep 17 00:00:00 2001 From: Roman Rizzi Date: Mon, 21 Oct 2019 13:14:15 -0300 Subject: [PATCH] FIX: Rate limit and hijack certificate generation. (#8215) To eliminate a DDOS attack vector, we're taking the following measures: The endpoint will be rate-limited to 3 requests every 60 seconds (per user). A 24 hours max-age cache header is sent with the response. The route will be hijacked to generate the certificate in the background. --- .../certificate_generator.rb | 46 ++++++++++--------- plugins/discourse-narrative-bot/plugin.rb | 37 +++++++++------ .../spec/lib/certificate_generator_spec.rb | 6 ++- .../requests/discobot_certificate_spec.rb | 3 +- 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/certificate_generator.rb b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/certificate_generator.rb index ad805ae8d9b..f5660854de9 100644 --- a/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/certificate_generator.rb +++ b/plugins/discourse-narrative-bot/lib/discourse_narrative_bot/certificate_generator.rb @@ -2,8 +2,9 @@ module DiscourseNarrativeBot class CertificateGenerator - def initialize(user, date) + def initialize(user, date, avatar_url) @user = user + @avatar_url = avatar_url date = begin @@ -21,29 +22,36 @@ module DiscourseNarrativeBot end def new_user_track - width = 538.583 # Default width for the SVG - ApplicationController.render(inline: File.read(File.expand_path('../templates/new_user.svg.erb', __FILE__)), - assigns: { width: width, - discobot_user: @discobot_user, - date: @date, - avatar_url: base64_image_link(avatar_url), - logo_group: logo_group(55, width, 350), - name: name }) + svg_default_width = 538.583 + logo_container = logo_group(55, svg_default_width, 350) + + ApplicationController.render(inline: read_template('new_user'), assigns: assign_options(svg_default_width, logo_container)) end def advanced_user_track - width = 722.8 # Default width for the SVG - ApplicationController.render(inline: File.read(File.expand_path('../templates/advanced_user.svg.erb', __FILE__)), - assigns: { width: width, - discobot_user: @discobot_user, - date: @date, - avatar_url: base64_image_link(avatar_url), - logo_group: logo_group(40, width, 280), - name: name }) + svg_default_width = 722.8 + logo_container = logo_group(40, svg_default_width, 280) + + ApplicationController.render(inline: read_template('advanced_user'), assigns: assign_options(svg_default_width, logo_container)) end private + def read_template(filename) + File.read(File.expand_path("../templates/#{filename}.svg.erb", __FILE__)) + end + + def assign_options(width, logo_group) + { + width: width, + discobot_user: @discobot_user, + date: @date, + avatar_url: base64_image_link(@avatar_url), + logo_group: logo_group, + name: name + } + end + def name @user.username.titleize end @@ -84,9 +92,5 @@ module DiscourseNarrativeBot rescue OpenURI::HTTPError # Ignore if fetching image returns a non 200 response end - - def avatar_url - UrlHelper.absolute(Discourse.base_uri + @user.avatar_template.gsub('{size}', '250')) - end end end diff --git a/plugins/discourse-narrative-bot/plugin.rb b/plugins/discourse-narrative-bot/plugin.rb index 02d78d05e75..00058f8c31b 100644 --- a/plugins/discourse-narrative-bot/plugin.rb +++ b/plugins/discourse-narrative-bot/plugin.rb @@ -80,31 +80,38 @@ after_initialize do requires_login def generate - unless params[:user_id]&.present? - raise Discourse::InvalidParameters.new('user_id must be present') + immutable_for(24.hours) + + %i[date user_id].each do |key| + raise Discourse::InvalidParameters.new("#{key} must be present") unless params[key]&.present? end + rate_limiter = RateLimiter.new(current_user, 'svg_certificate', 3, 1.minute) + rate_limiter.performed! unless current_user.staff? + user = User.find_by(id: params[:user_id]) raise Discourse::NotFound if user.blank? + cdn_avatar_url = fetch_avatar_url(user) - unless params[:date]&.present? - raise Discourse::InvalidParameters.new('date must be present') - end + hijack do + generator = CertificateGenerator.new(user, params[:date], cdn_avatar_url) - generator = CertificateGenerator.new(user, params[:date]) + svg = params[:type] == 'advanced' ? generator.advanced_user_track : generator.new_user_track - svg = - case params[:type] - when 'advanced' - generator.advanced_user_track - else - generator.new_user_track + respond_to do |format| + format.svg { render inline: svg } end - - respond_to do |format| - format.svg { render inline: svg } end end + + private + + def fetch_avatar_url(user) + avatar_url = UrlHelper.absolute(Discourse.base_uri + user.avatar_template.gsub('{size}', '250')) + URI(avatar_url).open('rb', redirect: false).read + rescue OpenURI::HTTPError + # Ignore if fetching image returns a non 200 response + end end end diff --git a/plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb b/plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb index c6cd3a50f2a..991cd082b30 100644 --- a/plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb +++ b/plugins/discourse-narrative-bot/spec/lib/certificate_generator_spec.rb @@ -4,10 +4,12 @@ require 'rails_helper' RSpec.describe DiscourseNarrativeBot::CertificateGenerator do let(:user) { Fabricate(:user) } + let(:avatar_url) { 'http://test.localhost/cdn/avatar.png' } + let(:date) { "2017-00-10" } describe 'when an invalid date is given' do it 'should default to the current date' do - expect { described_class.new(user, "2017-00-10") }.to_not raise_error + expect { described_class.new(user, date, avatar_url) }.to_not raise_error end end @@ -19,7 +21,7 @@ RSpec.describe DiscourseNarrativeBot::CertificateGenerator do end it 'should not try to fetch a image' do - expect(described_class.new(user, "2017-02-10").send(:logo_group, 1, 1, 1)) + expect(described_class.new(user, date, avatar_url).send(:logo_group, 1, 1, 1)) .to eq(nil) end end diff --git a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb index 805838b09cb..d30c76cf6ad 100644 --- a/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb +++ b/plugins/discourse-narrative-bot/spec/requests/discobot_certificate_spec.rb @@ -27,7 +27,8 @@ describe "Discobot Certificate" do end it 'should return the right text' do - stub_request(:get, /letter_avatar_proxy/).to_return(status: 200) + stub_request(:get, /letter_avatar_proxy/).to_return(status: 200, body: 'http://test.localhost/cdn/avatar.png') + stub_request(:get, /avatar.png/).to_return(status: 200) stub_request(:get, SiteSetting.site_logo_small_url) .to_return(status: 200)