From f53ab56e923408b7f51e9b756e6088e4413cb65e Mon Sep 17 00:00:00 2001 From: cktricky Date: Thu, 14 Nov 2013 11:06:27 -0500 Subject: [PATCH] fixes a bug introduced during the transition from info_disclosure to A6 --- app/controllers/tutorials_controller.rb | 46 +-------- app/views/layouts/tutorial/_sidebar.html.erb | 3 - .../info_disclosure/_ssn_disclosure.html.erb | 98 ------------------- app/views/tutorials/info_disclosure.html.erb | 18 ---- config/routes.rb | 1 - ...ure_spec.rb => sensitive_data_exposure.rb} | 2 +- 6 files changed, 2 insertions(+), 166 deletions(-) delete mode 100644 app/views/layouts/tutorial/info_disclosure/_ssn_disclosure.html.erb delete mode 100644 app/views/tutorials/info_disclosure.html.erb rename spec/vulnerabilities/{info_disclosure_spec.rb => sensitive_data_exposure.rb} (91%) diff --git a/app/controllers/tutorials_controller.rb b/app/controllers/tutorials_controller.rb index 0b88a9e..a8d151c 100755 --- a/app/controllers/tutorials_controller.rb +++ b/app/controllers/tutorials_controller.rb @@ -83,51 +83,7 @@ class TutorialsController < ApplicationController def guard end - def info_disclosure - @bad_code_1 = - %q{ - - - - - - - - - - - - - - - - - - - - - - - -
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 %>
- } - - @good_code_1 = %q{ - class WorkInfo < ActiveRecord::Base - attr_accessible :DoB, :SSN, :bonuses, :income, :years_worked - belongs_to :user - - # We should probably use this - def last_four - "***-**-" << self.SSN[-4,4] - end - - end - } - - @bad_code_2 = %q{<%= @user.work_info.SSN %>} - @good_code_2 = %q{<%= @user.work_info.last_four %>} - end + def mass_assignment end diff --git a/app/views/layouts/tutorial/_sidebar.html.erb b/app/views/layouts/tutorial/_sidebar.html.erb index acfdfd7..b46902b 100755 --- a/app/views/layouts/tutorial/_sidebar.html.erb +++ b/app/views/layouts/tutorial/_sidebar.html.erb @@ -106,9 +106,6 @@ -
  • - <%= link_to "Info Dislosure", info_disclosure_tutorials_path %> -
  • <%= link_to "Mass Assignment", mass_assignment_tutorials_path %>
    • diff --git a/app/views/layouts/tutorial/info_disclosure/_ssn_disclosure.html.erb b/app/views/layouts/tutorial/info_disclosure/_ssn_disclosure.html.erb deleted file mode 100644 index 76c7d12..0000000 --- a/app/views/layouts/tutorial/info_disclosure/_ssn_disclosure.html.erb +++ /dev/null @@ -1,98 +0,0 @@ -
    -
    -
    - Information Disclosure (Sensitive) -
    -
    -
    -
    -
    -
    - - - - Description - -
    -
    -
    -

    - The application stores and returns full social security numbers. The clear-text storage of this value within the database falls under <%= link_to "Insecure Cryptographic Storage", crypto_tutorials_path, {:style => "color: rgb(181, 121, 158)"}%>. However, the other failure here is that the application returns this full SSN value within the response for the user's Work Info page. Although a portion of the SSN value is obfuscated using JavaScript (when rendered in the browser), any attacker who has positioned themselves to sniff this traffic or read the user's browser cache can extract the full value from the source. -

    -
    -
    -
    -
    -
    - - - - Bug - -
    -
    -
    -

    - The bug is introduced within app/views/work_info/index.html.erb, seen on line 20: -

    -

    - 

    -						<%= @bad_code_1 %>
-
    - - The value, stored unencrypted, is called directly from the database. (line 20) -

    -
    -
    -
    -
    -
    - - - - Solution - -
    -
    -
    -

    - A model method to return only the last four digits already exists. The following code was taken from the WorkInfo model - app/models/work_info.rb: -

    -

    - 

    -					<%= @good_code_1%>
-
    -

    -

    - Essentially, this takes the SSN string from the DB, retrieves only the last four characters in the string, and concatenates the last four characters with asterisks. Because this occurs at the model level, the view page never calls the full SSN value and therefore the user's browser never receives the full SSN. The view code would need to change from... - 

    -					<%= @bad_code_2 %>
-
    - to... - 
    -					<%= @good_code_2 %>
-
    -

    - -
    -
    -
    -
    -
    - - - - Hint - -
    -
    -
    -

    - Inspect your work information closely -

    -
    -
    -
    -
    -
    -
    \ No newline at end of file diff --git a/app/views/tutorials/info_disclosure.html.erb b/app/views/tutorials/info_disclosure.html.erb deleted file mode 100644 index b482a01..0000000 --- a/app/views/tutorials/info_disclosure.html.erb +++ /dev/null @@ -1,18 +0,0 @@ -
    -
    -
    -
    - <%= render :partial => "layouts/tutorial/info_disclosure/ssn_disclosure"%> -
    -
    -
    -
    - - \ No newline at end of file diff --git a/config/routes.rb b/config/routes.rb index 800d3d7..dac6707 100755 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,7 +52,6 @@ Railsgoat::Application.routes.draw do get "ssl_tls" get "redirects" get "guard" - get "info_disclosure" get "mass_assignment" get "constantize" get "gauntlt" diff --git a/spec/vulnerabilities/info_disclosure_spec.rb b/spec/vulnerabilities/sensitive_data_exposure.rb similarity index 91% rename from spec/vulnerabilities/info_disclosure_spec.rb rename to spec/vulnerabilities/sensitive_data_exposure.rb index ce0bd2a..bc1e72e 100644 --- a/spec/vulnerabilities/info_disclosure_spec.rb +++ b/spec/vulnerabilities/sensitive_data_exposure.rb @@ -1,6 +1,6 @@ require 'spec_helper' -feature 'sensitive information disclosure' do +feature 'sensitive data exposure' do before do UserFixture.reset_all_users @normal_user = UserFixture.normal_user