From 08165b714366b8db3bb1f387efa1dadf01863c98 Mon Sep 17 00:00:00 2001 From: Marcelo Júnior Date: Fri, 28 Aug 2015 14:36:45 -0300 Subject: [PATCH] Removing search controller vulnabilities --- app/controllers/public/search_controller.rb | 6 +++++- config/application.rb | 6 ++++++ config/initializers/sanitizer.rb | 14 -------------- lib/sanitize_params.rb | 66 +++++++++++++++++++++++++++++++++--------------------------------- test/functional/search_controller_test.rb | 24 ++++++++++++++++++++++++ 5 files changed, 68 insertions(+), 48 deletions(-) delete mode 100644 config/initializers/sanitizer.rb diff --git a/app/controllers/public/search_controller.rb b/app/controllers/public/search_controller.rb index a9d31bd..1718518 100644 --- a/app/controllers/public/search_controller.rb +++ b/app/controllers/public/search_controller.rb @@ -3,7 +3,10 @@ class SearchController < PublicController helper TagsHelper include SearchHelper include ActionView::Helpers::NumberHelper + include SanitizeParams + + before_filter :sanitize_params before_filter :redirect_asset_param, :except => [:assets, :suggestions] before_filter :load_category, :except => :suggestions before_filter :load_search_assets, :except => :suggestions @@ -134,7 +137,8 @@ class SearchController < PublicController def tag @tag = params[:tag] - @tag_cache_key = "tag_#{CGI.escape(@tag.to_s)}_env_#{environment.id.to_s}_page_#{params[:npage]}" + tag_str = @tag.kind_of?(Array) ? @tag.join(" ") : @tag.to_str + @tag_cache_key = "tag_#{CGI.escape(tag_str)}_env_#{environment.id.to_s}_page_#{params[:npage]}" if is_cache_expired?(@tag_cache_key) @searches[@asset] = {:results => environment.articles.tagged_with(@tag).paginate(paginate_options)} end diff --git a/config/application.rb b/config/application.rb index 8394adf..8e0041d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -15,6 +15,12 @@ module Noosfero require 'noosfero/plugin' + config.action_view.sanitized_allowed_tags |= %w(object embed param table + tr th td applet comment iframe audio video source) + config.action_view.sanitized_allowed_attributes |= %w(align border alt + vspace hspace width heigth value type data style target codebase archive + classid code flashvars scrolling frameborder controls autoplay colspan) + require 'noosfero/multi_tenancy' config.middleware.use Noosfero::MultiTenancy::Middleware diff --git a/config/initializers/sanitizer.rb b/config/initializers/sanitizer.rb deleted file mode 100644 index fa62ce7..0000000 --- a/config/initializers/sanitizer.rb +++ /dev/null @@ -1,14 +0,0 @@ -require 'loofah/helpers' - -ActionView::Base.full_sanitizer = Loofah::Helpers::ActionView::FullSanitizer.new -ActionView::Base.white_list_sanitizer = Loofah::Helpers::ActionView::WhiteListSanitizer.new - -Loofah::HTML5::WhiteList::ALLOWED_ELEMENTS_WITH_LIBXML2.merge %w[ - img object embed param table tr th td applet comment iframe audio video source -] - -Loofah::HTML5::WhiteList::ALLOWED_ATTRIBUTES.merge %w[ - align border alt vspace hspace width heigth value type data - style target codebase archive classid code flashvars scrolling frameborder controls autoplay colspan -] - diff --git a/lib/sanitize_params.rb b/lib/sanitize_params.rb index 676bc6a..20044a4 100644 --- a/lib/sanitize_params.rb +++ b/lib/sanitize_params.rb @@ -2,40 +2,40 @@ module SanitizeParams protected - # Check each request parameter for - # improper HTML or Script tags - def sanitize_params - sanitize_params_hash(request.params) - end + # Check each request parameter for + # improper HTML or Script tags + def sanitize_params + sanitize_params_hash(params) + end - # Given a params list sanitize all - def sanitize_params_hash(params) - params.each { |k, v| - if v.is_a?(String) - params[k] = sanitize_param v - elsif v.is_a?(Array) - params[k] = sanitize_array v - elsif v.kind_of?(Hash) - params[k] = sanitize_params_hash(v) - end - } - end + # Given a params list sanitize all + def sanitize_params_hash(params) + params.each { |k, v| + if v.is_a?(String) + params[k] = sanitize_param v + elsif v.is_a?(Array) + params[k] = sanitize_array v + elsif v.kind_of?(Hash) + params[k] = sanitize_params_hash(v) + end + } + end - # If the parameter was an array, - # try to sanitize each element in the array - def sanitize_array(array) - array.map! { |e| - if e.is_a?(String) - sanitize_param e - end - } - return array - end + # If the parameter was an array, + # try to sanitize each element in the array + def sanitize_array(array) + array.map! { |e| + if e.is_a?(String) + sanitize_param e + end + } + return array + end - # Santitize a single value - def sanitize_param(value) - allowed_tags = %w(a acronym b strong i em li ul ol h1 h2 h3 h4 h5 h6 blockquote br cite sub sup ins p) - ActionController::Base.helpers.sanitize(value, tags: allowed_tags, attributes: %w(href title)) - end + # Santitize a single value + def sanitize_param(value) + allowed_tags = %w(a acronym b strong i em li ul ol h1 h2 h3 h4 h5 h6 blockquote br cite sub sup ins p) + ActionController::Base.helpers.sanitize(value, tags: allowed_tags, attributes: %w(href title)) + end -end +end diff --git a/test/functional/search_controller_test.rb b/test/functional/search_controller_test.rb index 92b7d61..5ab4543 100644 --- a/test/functional/search_controller_test.rb +++ b/test/functional/search_controller_test.rb @@ -779,6 +779,30 @@ class SearchControllerTest < ActionController::TestCase assert_equivalent [t1,t2,c1,c2,c3,c4] , assigns(:searches)[:communities][:results] end + should 'not raise an exception if tag query contains accented latin characters' do + tag_query = 'àáâãäå' + assert_nothing_raised(NoMethodError) { get :tag, :tag => tag_query } + end + + should 'not allow query injection' do + injection = 'SearchParam' + get :tag, :tag => injection + tag = assigns(:tag) + assert !tag.upcase.include?('IMG') + assert tag.include?('SearchParam') + end + + should 'not allow query injection in array' do + injection = ['', + ''] + get :tag, :tag => injection + tag = assigns(:tag) + tag.each { |t| + assert !t.upcase.include?('IMG') + assert !t.upcase.include?('SCRIPT') + } + end + protected def create_event(profile, options) -- libgit2 0.21.2