From 5643edcc5da84f8338e2b48547f8de6765735e48 Mon Sep 17 00:00:00 2001
From: Joseph Mastey <jmmastey@gmail.com>
Date: Tue, 19 Sep 2017 22:16:05 -0500
Subject: [PATCH] refactor vulnerabilities so that users can turn them from
 failing to passing

---
 spec/vulnerabilities/broken_auth_spec.rb      | 31 +++++++-----
 .../vulnerabilities/command_injection_spec.rb | 14 +++---
 spec/vulnerabilities/csrf_spec.rb             | 11 +++--
 spec/vulnerabilities/insecure_dor_spec.rb     | 32 ++++++-------
 spec/vulnerabilities/mass_assignment_spec.rb  | 30 ++++++------
 .../password_complexity_spec.rb               | 17 ++++---
 spec/vulnerabilities/password_hashing_spec.rb | 14 +++---
 .../sensitive_data_exposure.rb                | 16 ++++---
 spec/vulnerabilities/sql_injection_spec.rb    | 47 +++++--------------
 .../unvalidated_redirects_spec.rb             | 13 ++---
 spec/vulnerabilities/url_access_spec.rb       | 11 +++--
 spec/vulnerabilities/xss_spec.rb              | 21 +++++----
 12 files changed, 130 insertions(+), 127 deletions(-)

diff --git a/spec/vulnerabilities/broken_auth_spec.rb b/spec/vulnerabilities/broken_auth_spec.rb
index 5af2a7d..65b4775 100644
--- a/spec/vulnerabilities/broken_auth_spec.rb
+++ b/spec/vulnerabilities/broken_auth_spec.rb
@@ -2,34 +2,39 @@
 require "spec_helper"
 
 feature "broken_auth" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+
+    pending unless verifying_fixed?
   end
 
   scenario "one\nTutorial: https://github.com/OWASP/railsgoat/wiki/A2-Credential-Enumeration" do
-    visit "/"
-    within(".signup") do
-      fill_in "email", with: @normal_user.email + "not"
-      fill_in "password", with: @normal_user.clear_password
+    wrong_email = normal_user.email + "not"
+
+    visit '/'
+    within('.signup') do
+      fill_in 'email', with: wrong_email
+      fill_in 'password', with: normal_user.clear_password
     end
     within(".actions") do
       click_on "Login"
     end
-    pending if verifying_fixed?
-    expect(find("div#flash_notice").text).to eq("#{@normal_user.email}not doesn't exist!")
+
+    expect(find('div#flash_notice').text).not_to include(wrong_email)
   end
 
   scenario "two\nTutorial: https://github.com/OWASP/railsgoat/wiki/A2-Credential-Enumeration" do
-    visit "/"
-    within(".signup") do
-      fill_in "email", with: @normal_user.email
-      fill_in "password", with: @normal_user.clear_password + "not"
+    visit '/'
+    within('.signup') do
+      fill_in 'email', with: normal_user.email
+      fill_in 'password', with: normal_user.clear_password + 'not'
     end
     within(".actions") do
       click_on "Login"
     end
-    pending if verifying_fixed?
-    expect(find("div#flash_notice").text).to eq("Incorrect Password!")
+
+    expect(find('div#flash_notice').text).not_to include('Incorrect Password!')
   end
 end
diff --git a/spec/vulnerabilities/command_injection_spec.rb b/spec/vulnerabilities/command_injection_spec.rb
index baa17ef..1249cd3 100644
--- a/spec/vulnerabilities/command_injection_spec.rb
+++ b/spec/vulnerabilities/command_injection_spec.rb
@@ -3,28 +3,30 @@ require "spec_helper"
 require "tmpdir"
 
 feature "command injection" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A1-Command-Injection", js: true do
-    login @normal_user
+    login(normal_user)
 
     legit_file = File.join(Rails.root, "public", "data", "legit.txt")
     File.open(legit_file, "w") { |f| f.puts "totes legit" }
 
-    visit "/users/#{@normal_user.id}/benefit_forms"
+    visit "/users/#{normal_user.user_id}/benefit_forms"
     Dir.mktmpdir do |dir|
       hackety_file = File.join(dir, "test; cd public && cd data && rm -f * ;")
       File.open(hackety_file, "w") { |f| f.print "mwahaha" }
       within(".new_benefits") do
         attach_file "benefits_upload", hackety_file
-        find(:xpath, "//input[@id='benefits_backup']", visible: false).set "true"
+        find(:xpath, "//input[@id='benefits_backup']", visible: false).set 'true'
       end
       click_on "Start Upload"
     end
-    pending if verifying_fixed?
-    expect(File.exist?(legit_file)).to be_falsey
+
+    expect(File.exists?(legit_file)).to be_truthy
   end
 end
diff --git a/spec/vulnerabilities/csrf_spec.rb b/spec/vulnerabilities/csrf_spec.rb
index a8b68f7..375ae40 100644
--- a/spec/vulnerabilities/csrf_spec.rb
+++ b/spec/vulnerabilities/csrf_spec.rb
@@ -3,9 +3,11 @@ require "spec_helper"
 require "tmpdir"
 
 feature "csrf" do
-  before do
+  let(:normal_user) { UserFixture.normal_user }
+
+  before(:each) do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/R5-A8-CSRF", js: true do
@@ -13,7 +15,7 @@ feature "csrf" do
     # TODO: is there a way to get this without visiting root first?
     base_url = current_url
 
-    login @normal_user
+    login(normal_user)
 
     Dir.mktmpdir do |dir|
       hackety_file = File.join(dir, "form.on.bad.guy.site.html")
@@ -40,7 +42,6 @@ feature "csrf" do
       end
     end
 
-    pending if verifying_fixed?
-    expect(@normal_user.reload.paid_time_off.schedule.last.event_name).to eq("Bad Guy")
+    expect(normal_user.reload.paid_time_off.schedule.last.event_name).not_to eq("Bad Guy")
   end
 end
diff --git a/spec/vulnerabilities/insecure_dor_spec.rb b/spec/vulnerabilities/insecure_dor_spec.rb
index e0743ea..018aace 100644
--- a/spec/vulnerabilities/insecure_dor_spec.rb
+++ b/spec/vulnerabilities/insecure_dor_spec.rb
@@ -2,33 +2,31 @@
 require "spec_helper"
 
 feature "insecure direct object reference" do
+  let(:normal_user) { UserFixture.normal_user }
+  let(:another_user) { User.find_by(user_id: 2) }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
-  scenario "attack one" do
-    login(@normal_user)
+  scenario 'attack one' do
+    login(normal_user)
 
-    visit "/users/#{@normal_user.id}/benefit_forms"
-    download_url = first(".widget-body a")[:href]
-    visit download_url.sub(/name=(.*?)&/, "name=config/database.yml&")
+    visit "/users/#{normal_user.user_id}/benefit_forms"
+    download_url = first('.widget-body a')[:href]
+    visit download_url.sub(/name=(.*?)&/, 'name=config/database.yml&')
 
-    pending if verifying_fixed?
-
-    expect(page.status_code).to eq(200)
-    expect(page.response_headers["Content-Disposition"]).to include("database.yml")
-    expect(page.response_headers["Content-Length"]).to eq("710")
+    expect(page.status_code).not_to eq(200)
+    expect(page.response_headers['Content-Disposition']).not_to include('database.yml')
   end
 
   scenario "attack two\nTutorial: https://github.com/OWASP/railsgoat/wiki/A4-Insecure-Direct-Object-Reference" do
-    login(@normal_user)
-    expect(@normal_user.id).not_to eq(2)
-    another_user = User.find(2)
+    expect(normal_user.user_id).not_to eq(another_user.user_id)
 
-    visit "/users/#{another_user.id}/work_info"
+    visit "/users/#{another_user.user_id}/work_info"
 
-    pending if verifying_fixed?
-    expect(first("td").text).to eq(another_user.full_name)
+    expect(first('td').text).not_to include(another_user.name)
+    expect(first('td').text).to include(normal_user.name)
   end
 end
diff --git a/spec/vulnerabilities/mass_assignment_spec.rb b/spec/vulnerabilities/mass_assignment_spec.rb
index 2e27350..9657576 100644
--- a/spec/vulnerabilities/mass_assignment_spec.rb
+++ b/spec/vulnerabilities/mass_assignment_spec.rb
@@ -2,37 +2,37 @@
 require "spec_helper"
 
 feature "mass assignment" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
   scenario "attack one" do
-    expect(@normal_user.admin).to be_falsey
+    expect(normal_user.admin).to be_falsey
+    login(normal_user)
 
-    login(@normal_user)
+    params = { user: { admin: "t",
+                        user_id: normal_user.user_id,
+                        password: normal_user.clear_password,
+                        password_confirmation: normal_user.clear_password }}
 
-    params = { user:  { admin: "t",
-                        id: @normal_user.id,
-                        password: @normal_user.clear_password,
-                        password_confirmation: @normal_user.clear_password}}
-    page.driver.put "/users/#{@normal_user.id}.json", params
+    page.driver.put "/users/#{normal_user.user_id}.json", params
 
-    pending if verifying_fixed?
-    expect(@normal_user.reload.admin).to be_truthy
+    expect(normal_user.reload.admin).to be_falsy
   end
 
   scenario "attack two, Tutorial: https://github.com/OWASP/railsgoat/wiki/R5-Extras-Mass-Assignment-Admin-Role" do
-    params = {user: {admin: "t",
+    params = { user: {  admin: "t",
                         email: "hackety@h4x0rs.c0m",
                         first_name: "hackety",
                         last_name: "hax",
                         password: "foobarewe",
-                        password_confirmation: "foobarewe"}}
+                        password_confirmation: "foobarewe" }}
+
     page.driver.post "/users", params
 
-    pending if verifying_fixed?
-    expect(User.last.email).to eq("hackety@h4x0rs.c0m")
-    expect(User.last.admin).to be_truthy
+    expect(User.find_by(email: "hackety@h4x0rs.c0m")).to be_nil
   end
 end
diff --git a/spec/vulnerabilities/password_complexity_spec.rb b/spec/vulnerabilities/password_complexity_spec.rb
index 5a2bf7c..79da0a2 100644
--- a/spec/vulnerabilities/password_complexity_spec.rb
+++ b/spec/vulnerabilities/password_complexity_spec.rb
@@ -2,22 +2,27 @@
 require "spec_helper"
 
 feature "password complexity" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
   scenario "one\nTutorial: https://github.com/OWASP/railsgoat/wiki/A2-Lack-of-Password-Complexity" do
+    new_user_email = normal_user.email + "two"
+
     visit "/signup"
     within(".signup") do
-      fill_in "user_email", with: @normal_user.email + "not"
-      fill_in "user_first_name", with: @normal_user.first_name
-      fill_in "user_last_name", with: @normal_user.last_name + "not"
+      fill_in "user_email", with: new_user_email
+      fill_in "user_first_name", with: normal_user.first_name
+      fill_in "user_last_name", with: normal_user.last_name + "not"
       fill_in "user_password", with: "password"
       fill_in "user_password_confirmation", with: "password"
     end
     click_on "Submit"
-    pending if verifying_fixed?
-    expect(current_path).to eq("/dashboard/home")
+
+    expect(User.find_by(email: new_user_email)).to be_nil
+    expect(current_path).to eq("/signup")
   end
 end
diff --git a/spec/vulnerabilities/password_hashing_spec.rb b/spec/vulnerabilities/password_hashing_spec.rb
index d438409..7fd2c10 100644
--- a/spec/vulnerabilities/password_hashing_spec.rb
+++ b/spec/vulnerabilities/password_hashing_spec.rb
@@ -2,18 +2,20 @@
 require "spec_helper"
 
 feature "improper password hashing" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+    pending unless verifying_fixed?
   end
 
   scenario "with just md5\nTutorial: https://github.com/OWASP/railsgoat/wiki/A6-Sensitive-Data-Exposure-Insecure-Password-Storage" do
     new_pass = "testPassw0rd!"
-    @normal_user.password = new_pass
-    @normal_user.password_confirmation = new_pass
-    @normal_user.save
-    pending if verifying_fixed?
-    expect(Digest::MD5.hexdigest(new_pass)).to eq(@normal_user.password)
+    normal_user.password = new_pass
+    normal_user.password_confirmation = new_pass
+    normal_user.save!
+
+    expect(normal_user.password).not_to eq(Digest::MD5.hexdigest(new_pass))
   end
 
 end
diff --git a/spec/vulnerabilities/sensitive_data_exposure.rb b/spec/vulnerabilities/sensitive_data_exposure.rb
index d1a2007..0d4fb19 100644
--- a/spec/vulnerabilities/sensitive_data_exposure.rb
+++ b/spec/vulnerabilities/sensitive_data_exposure.rb
@@ -2,19 +2,23 @@
 require "spec_helper"
 
 feature "sensitive data exposure" do
+  let(:normal_user) { UserFixture.normal_user }
+  let(:user_ssn) { "999-99-9999" }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
-    @normal_user.work_info.update_attribute(:SSN, "999-99-9999")
+    normal_user.work_info.update_attribute(:SSN, user_ssn)
+
+    pending unless verifying_fixed?
   end
 
   # this won't work with javascript_driver, as it'll apply the javascript
   # function to mask this value and the source will be overwritten.
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A6-Sensitive-Data-Exposure-Cleartext-Storage-SSNs" do
-    login @normal_user
+    login(normal_user)
 
-    visit "/users/#{@normal_user.id}/work_info"
-    pending if verifying_fixed?
-    expect(page.source).to include "999-99-9999"
+    visit "/users/#{normal_user.user_id}/work_info"
+
+    expect(page.source).not_to include(user_ssn)
   end
 end
diff --git a/spec/vulnerabilities/sql_injection_spec.rb b/spec/vulnerabilities/sql_injection_spec.rb
index cf6de19..1697d7b 100644
--- a/spec/vulnerabilities/sql_injection_spec.rb
+++ b/spec/vulnerabilities/sql_injection_spec.rb
@@ -2,52 +2,31 @@
 require "spec_helper"
 
 feature "sql injection" do
-  before(:each) do
+  let(:normal_user) { UserFixture.normal_user }
+  let(:admin_user) { User.where(admin: true).first }
+
+  before do
     UserFixture.reset_all_users
-
-    @normal_user = UserFixture.normal_user
-    @admin_user = UserFixture.admin_user
+    pending unless verifying_fixed?
   end
-  before(:each) { pending unless verifying_fixed? }
 
-  scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/R4-A1-SQL-Injection-Concatentation" do
-    expect(@admin_user.admin).to be_truthy
+  scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/R5-A1-SQL-Injection-Concatentation" do
+    expect(admin_user.admin).to be_truthy
 
-    login(@normal_user)
-
-    visit "/users/#{@normal_user.id}/account_settings"
+    login(normal_user)
 
+    visit "/users/#{normal_user.user_id}/account_settings"
     within("#account_edit") do
       fill_in "Email", with: "joe.admin@schmoe.com"
-      fill_in "user_password", with: "H4cketyhack"
-      fill_in "user_password_confirmation", with: "H4cketyhack"
+      fill_in "user_password", with: "hacketyhack"
+      fill_in "user_password_confirmation", with: "hacketyhack"
 
       # this is a hidden field, so cannot use fill_in to access it.
       find(:xpath, "//input[@id='user_id']", visible: false).set "8' OR admin='t') --"
     end
     click_on "Submit"
 
-    @admin_user = User.where(admin: true).first
-    expect(@admin_user.email).not_to eq("joe.admin@schmoe.com")
-  end
-
-  scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A1-SQL-Injection-Interpolation", js: true do
-    login(@normal_user)
-    Analytics.create!(ip_address: "::1")
-
-    visit "/admin/1/analytics"
-
-    within("#analytics_search") do
-      fill_in "ip", with: "::1"
-      check "field_user_agent"
-      payload = "(select group_concat(password) from users where admin='t')"
-
-      page.execute_script "$('#field_user_agent').attr('name', \"field[#{payload}]\");"
-      page.execute_script "$('#analytics_search').submit();"
-    end
-
-    pending if verifying_fixed?
-    expect(page).to have_css(".dataTable.custom")
-    expect(page.source).to include(@admin_user.password)
+    admin_user = User.where(admin: true).first
+    expect(admin_user.email).not_to eq("joe.admin@schmoe.com")
   end
 end
diff --git a/spec/vulnerabilities/unvalidated_redirects_spec.rb b/spec/vulnerabilities/unvalidated_redirects_spec.rb
index c3ff7d7..a1ff494 100644
--- a/spec/vulnerabilities/unvalidated_redirects_spec.rb
+++ b/spec/vulnerabilities/unvalidated_redirects_spec.rb
@@ -2,23 +2,24 @@
 require "spec_helper"
 
 feature "unvalidated redirect" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+
+    pending unless verifying_fixed?
   end
 
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A10-Unvalidated-Redirects-and-Forwards-(redirect_to)", js: true do
     visit "/?url=http://example.com/do/evil/things"
-
     within(".signup") do
-      fill_in "email", with: @normal_user.email
-      fill_in "password", with: @normal_user.clear_password
+      fill_in "email", with: normal_user.email
+      fill_in "password", with: normal_user.clear_password
     end
     within(".actions") do
       click_on "Login"
     end
 
-    pending if verifying_fixed?
-    expect(current_url).to eq("http://example.com/do/evil/things")
+    expect(current_url).to eq("/dashboard/home")
   end
 end
diff --git a/spec/vulnerabilities/url_access_spec.rb b/spec/vulnerabilities/url_access_spec.rb
index b8f73e9..f7d0468 100644
--- a/spec/vulnerabilities/url_access_spec.rb
+++ b/spec/vulnerabilities/url_access_spec.rb
@@ -2,16 +2,19 @@
 require "spec_helper"
 
 feature "url access" do
+  let(:normal_user) { UserFixture.normal_user }
+
   before do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+
+    pending unless verifying_fixed?
   end
 
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A7-Missing-Function-Level-Access-Control--(Admin-Controller)", js: true do
-    login @normal_user
+    login(normal_user)
 
     visit "/admin/1/dashboard"
-    pending if verifying_fixed?
-    expect(current_path).to eq("/admin/1/dashboard")
+
+    expect(current_path).to eq("/")
   end
 end
diff --git a/spec/vulnerabilities/xss_spec.rb b/spec/vulnerabilities/xss_spec.rb
index 7a63d5f..001f09a 100644
--- a/spec/vulnerabilities/xss_spec.rb
+++ b/spec/vulnerabilities/xss_spec.rb
@@ -2,30 +2,33 @@
 require "spec_helper"
 
 feature "xss" do
-  before do
+  let(:normal_user) { UserFixture.normal_user }
+
+  before(:each) do
     UserFixture.reset_all_users
-    @normal_user = UserFixture.normal_user
+
+    pending unless verifying_fixed?
   end
 
   scenario "attack\nTutorial: https://github.com/OWASP/railsgoat/wiki/A3-Cross-Site-Scripting", js: true do
-    login @normal_user
+    login(normal_user)
 
-    visit "/users/#{@normal_user.id}/account_settings"
+    visit "/users/#{normal_user.user_id}/account_settings"
     within("#account_edit") do
       fill_in "First name", with: "<script>$(function() { $('div input.btn').val('RailsGoat h4x0r3d') } )</script>"
 
       # password gets screwed up if you don't re-submit - need to fix
-      fill_in "user_password", with: @normal_user.clear_password
-      fill_in "user_password_confirmation", with: @normal_user.clear_password
+      fill_in "user_password", with: normal_user.clear_password
+      fill_in "user_password_confirmation", with: normal_user.clear_password
     end
     click_on "Submit"
 
     sleep(1)
 
-    visit "/users/#{@normal_user.id}/account_settings"
+    visit "/users/#{normal_user.user_id}/account_settings"
 
-    pending if verifying_fixed?
-    expect(find("#submit_button").value).to eq("RailsGoat h4x0r3d")
+
+    expect(find("#submit_button").value).not_to include("RailsGoat h4x0r3d")
 
     # might be nice to demonstrate posting cookie contents or somesuch, but
     # this at least shows the vulnerability still exists.