From 45b88d565b2dad6e29e4f673d5bd8ecf63cb3d2d Mon Sep 17 00:00:00 2001 From: Yammy Date: Fri, 19 May 2017 11:20:40 +0800 Subject: [PATCH 1/2] test safe --- app/controllers/application_controller.rb | 2 +- app/controllers/events_controller.rb | 6 ++++-- app/controllers/users_controller.rb | 11 +++++++++-- app/helpers/users_helper.rb | 10 +++++++--- app/views/events/show.html.erb | 6 +++--- config/routes.rb | 2 +- 6 files changed, 25 insertions(+), 12 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 682a07d..cd6ea53 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,6 +1,6 @@ class ApplicationController < ActionController::Base - # protect_from_forgery with: :exception + protect_from_forgery with: :exception helper_method :current_cart diff --git a/app/controllers/events_controller.rb b/app/controllers/events_controller.rb index 245dece..4ab4b15 100644 --- a/app/controllers/events_controller.rb +++ b/app/controllers/events_controller.rb @@ -9,10 +9,12 @@ def show @comments = @event.comments if params[:keyword] - @comments = @comments.where( "comments.content LIKE '%#{params[:keyword]}%'") + # @comments = @comments.where( "comments.content LIKE '%#{params[:keyword]}%'") + keyword = ActiveRecord::Base::connection.quote_string( params[:keyword] ) + @comments = @comments.where( "comments.content LIKE ?", "%#{params[:keyword]}%") end - if params[:sort] + if params[:sort] && ["id DESC", "id ASC"].include?(params[:sort]) @comments = @comments.order(params[:sort]) end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index f4b4e65..1a5e5d5 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -10,13 +10,20 @@ def edit def update @user = current_user - params[:user].permit! + # params[:user].permit! - if @user.update(params[:user]) + # if @user.update(params[:user]) + if @user.update(user_params) redirect_to user_path(@user) else render "edit" end end + protected + + def user_params + params.require(:user).permit(:nickname) + end + end diff --git a/app/helpers/users_helper.rb b/app/helpers/users_helper.rb index ed49051..7c220a0 100644 --- a/app/helpers/users_helper.rb +++ b/app/helpers/users_helper.rb @@ -7,9 +7,13 @@ def user_avatar_link(user) email_md5 = Digest::MD5.hexdigest(user.email) gravatar_url = "https://www.gravatar.com/avatar/#{email_md5}" - str = "
" + link_to(image_tag(gravatar_url), user_path(user)) + " " + user.display_name + "
" - - str.html_safe + # src = "
" + link_to(image_tag(gravatar_url), user_path(user)) + " " + user.display_name + "
" + # src.html_safe + # + # "
".html_safe + link_to(image_tag(gravatar_url), user_path(user)) + " " + user.display_name + "
" + content_tag(:div, + link_to(image_tag(gravatar_url), user_path(user)) + tag("") + user.display_name, + :class => "user-link") end end diff --git a/app/views/events/show.html.erb b/app/views/events/show.html.erb index 0e89067..cfbe93a 100644 --- a/app/views/events/show.html.erb +++ b/app/views/events/show.html.erb @@ -22,14 +22,14 @@
- <%= raw comment.content %> + <%= sanitize comment.content %>
<%= comment.created_at.to_s %> <% if current_user && current_user.is_admin? %> - <%= link_to "Highligh", highlight_event_comment_path(@event, comment), :class => "btn btn-default" %> + <%= link_to "Highligh", highlight_event_comment_path(@event, comment), method: :post, :class => "btn btn-default" %> <% end %> <% if comment.can_deleted_by(current_user) %> @@ -50,4 +50,4 @@
<%= f.submit "Comment", :class => "btn btn-primary" %>
-<% end %> \ No newline at end of file +<% end %> diff --git a/config/routes.rb b/config/routes.rb index 2cfd938..a5ccf3e 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -7,7 +7,7 @@ resources :events do resources :comments do member do - get :highlight + post :highlight end end end From 1f7da07de9ede57c8e967deab615ee5ff6e4e11d Mon Sep 17 00:00:00 2001 From: Yammy Date: Fri, 19 May 2017 11:42:45 +0800 Subject: [PATCH 2/2] fix possible problem and gem security hole by installed brakeman and bundle-audit --- Gemfile | 5 ++- Gemfile.lock | 9 +++++ config/application.rb | 3 +- config/initializers/rack-attack.rb | 59 ++++++++++++++++++++++++++++++ 4 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 config/initializers/rack-attack.rb diff --git a/Gemfile b/Gemfile index e91c3ae..489202f 100644 --- a/Gemfile +++ b/Gemfile @@ -40,6 +40,8 @@ gem 'jbuilder', '~> 2.5' # Use Capistrano for deployment # gem 'capistrano-rails', group: :development +gem 'rack-attack' + group :development, :test do gem 'rspec-rails' # Call 'byebug' anywhere in the code to stop execution and get a debugger console @@ -48,7 +50,8 @@ end group :development do gem 'faker' - + gem 'brakeman' + gem 'bundler-audit' # Access an IRB console on exception pages or by using <%= console %> anywhere in the code. gem 'web-console', '>= 3.3.0' gem 'listen', '~> 3.0.5' diff --git a/Gemfile.lock b/Gemfile.lock index 30c443b..5e6563f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -46,7 +46,11 @@ GEM bootstrap-sass (3.3.7) autoprefixer-rails (>= 5.2.1) sass (>= 3.3.4) + brakeman (3.5.0) builder (3.2.3) + bundler-audit (0.5.0) + bundler (~> 1.2) + thor (~> 0.18) byebug (9.0.6) coffee-rails (4.2.1) coffee-script (>= 2.2.0) @@ -98,6 +102,8 @@ GEM orm_adapter (0.5.0) puma (3.8.2) rack (2.0.1) + rack-attack (5.0.1) + rack rack-test (0.6.3) rack (>= 1.0) rails (5.0.2) @@ -192,6 +198,8 @@ PLATFORMS DEPENDENCIES bootstrap-sass + brakeman + bundler-audit byebug coffee-rails (~> 4.2) devise @@ -200,6 +208,7 @@ DEPENDENCIES jquery-rails listen (~> 3.0.5) puma (~> 3.0) + rack-attack rails (~> 5.0.2) rspec-rails sass-rails (~> 5.0) diff --git a/config/application.rb b/config/application.rb index 2d9ee51..a8be62d 100644 --- a/config/application.rb +++ b/config/application.rb @@ -12,8 +12,9 @@ class Application < Rails::Application # Application configuration should go into files in config/initializers # -- all .rb files in that directory are automatically loaded. config.time_zone = "Beijing" + config.middleware.use Rack::Attack end end -Time::DATE_FORMATS.merge!(:default => '%Y/%m/%d %I:%M %p', :ymd => '%Y/%m/%d') \ No newline at end of file +Time::DATE_FORMATS.merge!(:default => '%Y/%m/%d %I:%M %p', :ymd => '%Y/%m/%d') diff --git a/config/initializers/rack-attack.rb b/config/initializers/rack-attack.rb new file mode 100644 index 0000000..61a40b9 --- /dev/null +++ b/config/initializers/rack-attack.rb @@ -0,0 +1,59 @@ +class Rack::Attack + + throttle('req/ip', :limit => 180, :period => 1.minutes) do |req| + req.ip + end + + ### Prevent Brute-Force Login Attacks ### + + + # The most common brute-force login attack is a brute-force password + + # attack where an attacker simply tries a large number of emails and + + # passwords to see if any credentials match. + + # + + # Another common method of attack is to use a swarm of computers with + + # different IPs to try brute-forcing a password for a specific account. + + + # Throttle POST requests to /login by IP address + + # + + # Key: "rack::attack:#{Time.now.to_i/:period}:logins/ip:#{req.ip}" + + throttle('logins/ip', :limit => 5, :period => 20.seconds) do |req| + if req.path == '/users/sign_in' && req.post? + req.ip + end + end + + # Throttle POST requests to /login by email param + + # + + # Key: "rack::attack:#{Time.now.to_i/:period}:logins/email:#{req.email}" + + # + + # Note: This creates a problem where a malicious user could intentionally + + # throttle logins for another user and force their login requests to be + + # denied, but that's not very common and shouldn't happen to you. (Knock + + # on wood!) + + throttle("logins/email", :limit => 5, :period => 20.seconds) do |req| + if req.path == '/users/sign_in' && req.post? + # return the email if present, nil otherwise + + req.params['email'].presence + end + end + +end