Merge branch 'master' of https://github.com/OWASP/railsgoat

README.md

@@ -97,9 +97,6 @@ To run just one spec:
 $ rails training SPEC=spec/vulnerabilities/sql_injection_spec.rb
 ```
 
-NOTE: As vulnerabilities are fixed in the application, these specs will not change to `passing`, but to `pending`.
-
-
 ## MySQL Environment
 
 By default in development mode Railsgoat runs with a SQLite database. There is an environment defined to use MySQL. For some of the SQL injection vulnerabilities to work you have to run the app with MySQL as the database. The following steps will setup and run Railsgoat to use MySQL. *MySQL must be installed and running before running these steps*
@@ -139,7 +136,7 @@ Alternatively, you can run MailCatcher in the foreground by running `mailcatcher
 
 ## Contributing
 
-As changes are made to the application, the Capybara RSpecs can be used to verify that the vulnerabilities in the application are still intact. To use them in this way, and have them `pass` instead of `fail`, set the `RAILSGOAT_MAINTAINER` environment variable.
+As changes are made to the application, the Capybara RSpecs can be used to verify that the vulnerabilities in the application are still intact. To use them in this way, and have them change to `pending` instead of `fail`, set the `RAILSGOAT_MAINTAINER` environment variable.
 
 Conversion to the OWASP Top Ten 2013 completed in November, 2013.
 

spec/models/benefits_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 require "spec_helper.rb"
 
-describe User do
+describe Benefits do
   before(:all) do
     UserFixture.reset_all_users
     DatabaseCleaner.strategy = :transaction

spec/support/capybara_shared.rb

@@ -26,7 +26,7 @@ def verifying_fixed?
 
     **NOTE: The RSpec pending feature is used to toggle the outcome of these specs
     between Training mode and RailsGoat Maintainer mode. When the vulnerabilities
-    are removed, the specs will not "pass," but rather go into a "pending" state.
+    are removed, the specs will pass instead. Try to get a fully passing suite.
     ******************************************************************************
 
     NOTICE

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
+    wrong_email = normal_user.email + "not"
+
     visit "/"
     within(".signup") do
-      fill_in "email", with: @normal_user.email + "not"
-      fill_in "password", with: @normal_user.clear_password
+      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"
+      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

spec/vulnerabilities/command_injection_spec.rb

@@ -3,18 +3,20 @@ 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.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" }
@@ -24,7 +26,7 @@ feature "command injection" do
       end
       click_on "Start Upload"
     end
-    pending if verifying_fixed?
-    expect(File.exist?(legit_file)).to be_falsey
+
+    expect(File.exist?(legit_file)).to be_truthy
   end
 end

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

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(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)
+    login(normal_user)
 
-    visit "/users/#{@normal_user.id}/benefit_forms"
+    visit "/users/#{normal_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.id).not_to eq(another_user.id)
 
     visit "/users/#{another_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

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",
+                        id: normal_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.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

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

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

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.id}/work_info"
+
+    expect(page.source).not_to include(user_ssn)
   end
 end

spec/vulnerabilities/sql_injection_spec.rb

@@ -2,53 +2,31 @@
 require "spec_helper"
 
 feature "sql injection" do
-  before(:each) do
-    UserFixture.reset_all_users
+  let(:normal_user) { UserFixture.normal_user }
+  let(:admin_user) { User.where(admin: true).first }
 
-    @normal_user = UserFixture.normal_user
-    @admin_user = UserFixture.admin_user
+  before do
+    UserFixture.reset_all_users
+    pending unless verifying_fixed?
   end
 
-  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.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"
 
-    pending if verifying_fixed?
-    @admin_user = User.where("admin='t'").first
-    expect(@admin_user.email).to eq("joe.admin@schmoe.com")
-    expect(@admin_user.admin).to eq(true)
-  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

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

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

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