From ef2b2e8e113515c020c55b755d31c115faa4a0a6 Mon Sep 17 00:00:00 2001 From: Ken Johnson Date: Tue, 4 Jun 2013 11:00:01 -0400 Subject: [PATCH] okay, finally got a working redirect vuln --- app/controllers/application_controller.rb | 3 +- app/controllers/sessions_controller.rb | 11 +- app/models/work_info.rb | 7 ++ .../layouts/tutorial/crypto/_ssn.html.erb | 101 ++++++++++++++++++ app/views/sessions/new.html.erb | 1 + app/views/tutorials/crypto.html.erb | 7 ++ 6 files changed, 124 insertions(+), 6 deletions(-) create mode 100644 app/views/layouts/tutorial/crypto/_ssn.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f0f5454..eba7470 100755 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -13,7 +13,8 @@ class ApplicationController < ActionController::Base end def authenticated - redirect_to root_url and reset_session if not current_user + path = request.fullpath.present? ? root_url(:url => request.fullpath) : root_url + redirect_to path and reset_session if not current_user end def is_admin? diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index b803b8e..3f84173 100755 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,28 +4,29 @@ class SessionsController < ApplicationController skip_before_filter :authenticated, :only => [:new, :create] def new - redirect_to home_dashboard_index_path if current_user + @url = params[:url] + redirect_to home_dashboard_index_path if current_user end def create - + path = params[:url].present? ? params[:url] : home_dashboard_index_path begin # Normalize the email address, why not user = User.authenticate(params[:email].to_s.downcase, params[:password]) + # @url = params[:url] rescue Exception => e end if user session[:user_id] = user.user_id if User.where(:user_id => user.user_id).exists? - redirect_to home_dashboard_index_path + redirect_to path else # Removed this code, just doesn't seem specific enough! # flash[:error] = "Either your username and password is incorrect" flash[:error] = e.message render "new" - end - + end end def destroy diff --git a/app/models/work_info.rb b/app/models/work_info.rb index 4a564e0..c3b70b6 100644 --- a/app/models/work_info.rb +++ b/app/models/work_info.rb @@ -1,10 +1,17 @@ class WorkInfo < ActiveRecord::Base attr_accessible :DoB, :SSN, :bonuses, :income, :years_worked belongs_to :user + #before_save :encrypt_ssn # We should probably use this def last_four "***-**-" << self.SSN[-4,4] end + def encrypt_ssn + end + + def decrypt_ssn + end + end diff --git a/app/views/layouts/tutorial/crypto/_ssn.html.erb b/app/views/layouts/tutorial/crypto/_ssn.html.erb new file mode 100644 index 0000000..725d54f --- /dev/null +++ b/app/views/layouts/tutorial/crypto/_ssn.html.erb @@ -0,0 +1,101 @@ +
+
+
+ A7 - Insecure Cryptographic Storage - Clear-text storage of SSN(s) +
+
+
+
+
+
+ + + + Description + +
+
+
+

+ The Railsgoat application stores Social Security Numbers in plain-text and because of this, it fails to adequately protect these numbers from theft. +

+
+
+
+
+
+ + + + Bug + +
+
+
+

+ The WorkInfo model (app/models/work_info.rb) is where the code to encrypt this data should be. However, as seen, is missing any routine to do so. +

+ 
+				class WorkInfo < ActiveRecord::Base
+				  attr_accessible :DoB, :SSN, :bonuses, :income, :years_worked
+				  belongs_to :user
+
+				  # We should probably use this
+				  def last_four
+				    "***-**-" << self.SSN[-4,4]
+				  end
+
+				end
+	
+
+ +
+
+
+
+
+ + + + Solution + +
+
+
+

Password Storage - SOLUTION

+

+ There is a lot of guidance on adequately protecting sensitive data at rest and using a layered defensive approach. Make no mistake, this should not be your sole means of securing sensitive data. That being said, there are at least four precautions that should be taken. +

  • The sensitive data is encrypted everywhere, including backups
    • +
  • Only authorized users can access decrypted copies of the data
    • +
  • Use a strong algorithm
    • +
  • Strong key is generated, protected from unauthorized access, and key change is planned for.

    • + One additional item to note with rails specifically, the framework makes it easy to determine the type of environment running, example: + 
    +					Rails.env.production?
+
    + ...or + 
    +					Rails.env.development?
+
    + This allows developers to easily create different keys for development and production and should be considered an asset to utilize. While development keys are usually stored within the source code of most rails applications, and developers with access to the repo can download those keys, the same should NOT hold true for production keys. +

    +
    +
    +
    +
    +
    + + + + Hint + +
    +
    +
    + How protected are those passwords in the database against cracking? +
    +
    +
    +
    +
    +
    \ No newline at end of file diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index 0b3b7d3..481763a 100755 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -13,6 +13,7 @@
    + <%= hidden_field_tag :url, @url%> <%= label_tag "Email Address" %> <%= text_field_tag :email, params[:email], {:class => "input input-block-level"} %> diff --git a/app/views/tutorials/crypto.html.erb b/app/views/tutorials/crypto.html.erb index e04656c..7bd24af 100755 --- a/app/views/tutorials/crypto.html.erb +++ b/app/views/tutorials/crypto.html.erb @@ -5,6 +5,13 @@ <%= render :partial => "layouts/tutorial/crypto/password_hashing" %>
    +