From 2dadad395b2ffed29061aa78a4b4ec20fd8e2670 Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 11 Nov 2016 23:55:30 -0600 Subject: [PATCH 1/4] Add test for setting dmail filters on other people. --- .../user/dmail_filters_controller_test.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 test/functional/maintenance/user/dmail_filters_controller_test.rb diff --git a/test/functional/maintenance/user/dmail_filters_controller_test.rb b/test/functional/maintenance/user/dmail_filters_controller_test.rb new file mode 100644 index 000000000..43b057d66 --- /dev/null +++ b/test/functional/maintenance/user/dmail_filters_controller_test.rb @@ -0,0 +1,40 @@ +require "test_helper" + +module Maintenance + module User + class DmailFiltersControllerTest < ActionController::TestCase + context "The dmail filters controller" do + setup do + @user1 = FactoryGirl.create(:user) + @user2 = FactoryGirl.create(:user) + CurrentUser.user = @user1 + CurrentUser.ip_addr = "127.0.0.1" + end + + teardown do + CurrentUser.user = nil + CurrentUser.ip_addr = nil + end + + context "update action" do + should "not allow a user to create a filter belonging to another user" do + @dmail = FactoryGirl.create(:dmail, :owner => @user1) + + params = { + :dmail_id => @dmail.id, + :dmail_filter => { + :words => "owned", + :user_id => @user2.id + } + } + + post :update, params, { :user_id => @user1.id } + + assert_not_equal("owned", @user2.reload.dmail_filter.try(&:words)) + assert_redirected_to(@dmail) + end + end + end + end + end +end From a16b91e2bf60054175ae65390d563be838e4a35c Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 11 Nov 2016 23:57:55 -0600 Subject: [PATCH 2/4] Fix exploit allowing dmail filters to be set on other users. Exploit: curl \ -u $USERNAME:$API_KEY \ -X PUT "http://danbooru.donmai.us/maintenance/user/dmail_filter.json?dmail_id=1" \ -d "dmail_filter[words]=owned&dmail_filter[user_id]=2" ...where dmail_id is any dmail you own (doesn't matter which) and user_id is the victim. --- app/controllers/maintenance/user/dmail_filters_controller.rb | 2 +- app/models/dmail_filter.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/maintenance/user/dmail_filters_controller.rb b/app/controllers/maintenance/user/dmail_filters_controller.rb index 1d7b06d00..50ca9cb8d 100644 --- a/app/controllers/maintenance/user/dmail_filters_controller.rb +++ b/app/controllers/maintenance/user/dmail_filters_controller.rb @@ -10,7 +10,7 @@ module Maintenance def update @dmail_filter = CurrentUser.dmail_filter || DmailFilter.new - @dmail_filter.update_attributes(params[:dmail_filter]) + @dmail_filter.update(params.require(:dmail_filter).permit(:words), :as => CurrentUser.role) flash[:notice] = "Filter updated" redirect_to(dmail_path(@dmail.id)) end diff --git a/app/models/dmail_filter.rb b/app/models/dmail_filter.rb index b35a9a238..9a2e52c6a 100644 --- a/app/models/dmail_filter.rb +++ b/app/models/dmail_filter.rb @@ -1,6 +1,6 @@ class DmailFilter < ActiveRecord::Base belongs_to :user - attr_accessible :user_id, :words, :as => [:moderator, :janitor, :gold, :member, :anonymous, :default, :builder, :admin] + attr_accessible :words, :as => [:moderator, :janitor, :gold, :member, :anonymous, :default, :builder, :admin] validates_presence_of :user before_validation :initialize_user From b0a0a32173dd95ac5a1d36178c44f461533de167 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 12 Nov 2016 00:01:41 -0600 Subject: [PATCH 3/4] API: support `PUT /maintenance/user/dmail_filter.json`. --- app/controllers/maintenance/user/dmail_filters_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/maintenance/user/dmail_filters_controller.rb b/app/controllers/maintenance/user/dmail_filters_controller.rb index 50ca9cb8d..f5a0ce855 100644 --- a/app/controllers/maintenance/user/dmail_filters_controller.rb +++ b/app/controllers/maintenance/user/dmail_filters_controller.rb @@ -3,6 +3,7 @@ module Maintenance class DmailFiltersController < ApplicationController before_filter :ensure_ownership before_filter :member_only + respond_to :html, :json, :xml def edit @dmail_filter = CurrentUser.dmail_filter || DmailFilter.new @@ -12,7 +13,7 @@ module Maintenance @dmail_filter = CurrentUser.dmail_filter || DmailFilter.new @dmail_filter.update(params.require(:dmail_filter).permit(:words), :as => CurrentUser.role) flash[:notice] = "Filter updated" - redirect_to(dmail_path(@dmail.id)) + respond_with(@dmail) end private From 47f663e00221d3c4affa55fb92d9d3f43fca534d Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 12 Nov 2016 01:04:09 -0600 Subject: [PATCH 4/4] Don't filter dmails from moderators (fix #2757). --- app/models/dmail_filter.rb | 2 +- test/unit/dmail_test.rb | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/models/dmail_filter.rb b/app/models/dmail_filter.rb index 9a2e52c6a..6f554ebdf 100644 --- a/app/models/dmail_filter.rb +++ b/app/models/dmail_filter.rb @@ -11,7 +11,7 @@ class DmailFilter < ActiveRecord::Base end def filtered?(dmail) - dmail.from.level <= User::Levels::MODERATOR && has_filter? && (dmail.body =~ regexp || dmail.title =~ regexp || dmail.from.name =~ regexp) + dmail.from.level < User::Levels::MODERATOR && has_filter? && (dmail.body =~ regexp || dmail.title =~ regexp || dmail.from.name =~ regexp) end def has_filter? diff --git a/test/unit/dmail_test.rb b/test/unit/dmail_test.rb index 5cc10c3df..f7d220277 100644 --- a/test/unit/dmail_test.rb +++ b/test/unit/dmail_test.rb @@ -38,6 +38,16 @@ class DmailTest < ActiveSupport::TestCase assert_equal(false, @recipient.has_mail?) end + should "be ignored when sender is a moderator" do + CurrentUser.scoped(FactoryGirl.create(:moderator_user), "127.0.0.1") do + @dmail = FactoryGirl.create(:dmail, :owner => @recipient, :body => "banned word here", :to => @recipient) + end + + assert_equal(false, !!@recipient.dmail_filter.filtered?(@dmail)) + assert_equal(false, @dmail.is_read?) + assert_equal(true, @recipient.has_mail?) + end + context "that is empty" do setup do @recipient.dmail_filter.update_attributes(:words => " ")