From 911a52ee83e104222f43d1059921935ab224f3c4 Mon Sep 17 00:00:00 2001
From: chrismo <chrismo@clabs.org>
Date: Tue, 1 Oct 2013 23:26:28 -0500
Subject: [PATCH] Add pending code to flip-flop results of specs.

This isn't the cleanest approach, but should be good for now.

Obviously, there are two contexts for these specs: one is from the
maintainer's standpoint, the other is from the trainee who is using
RailsGoat for training.

The maintainer wants all of these specs to pass, to ensure the
vulnerabilities are still functional as vulnerabilities.

The trainee could potentially use these specs (though reading the specs
contains spoilers) to track and verify their fixes.

I've wired in a pending block around each assertion that checks a method
to see what the result of the pending call would be. You can see
examples of how this works with conditions here:
https://www.relishapp.com/rspec/rspec-core/v/2-14/docs/pending/pending-examples

This means these specs will all fail now by default (the trainee
context), but will pass, when vulnerable, if the RAILSGOAT_MAINTAINER
env var is set.

The only flaw at the moment is that in the trainee context, fixing the
vulnerabilities will result in the specs going from failing to
_pending_, not passing (which makes sense, given how we're using RSpec's
pending functionality).

Maybe it'd be simpler/better to have a boolean toggle of our own somehow
wrap the assertions in blocks to do explicitly what we want (flip-flop
the result based on the context).
---
 spec/features/broken_auth_spec.rb           |  9 ++++++---
 spec/features/command_injection_spec.rb     |  4 ++--
 spec/features/csrf_spec.rb                  |  2 +-
 spec/features/info_disclosure_spec.rb       |  2 +-
 spec/features/insecure_dor_spec.rb          | 10 ++++++----
 spec/features/mass_assignment_spec.rb       | 21 ++++++++++++++++++---
 spec/features/sql_injection_spec.rb         |  8 +++++---
 spec/features/unvalidated_redirects_spec.rb |  2 +-
 spec/features/url_access_spec.rb            |  2 +-
 spec/features/xss_spec.rb                   |  2 +-
 spec/support/capybara_shared.rb             | 11 +++++++++++
 11 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/spec/features/broken_auth_spec.rb b/spec/features/broken_auth_spec.rb
index d1f7e6e..4c83de0 100644
--- a/spec/features/broken_auth_spec.rb
+++ b/spec/features/broken_auth_spec.rb
@@ -6,20 +6,23 @@ feature 'broken_auth' do
     @normal_user = UserFixture.normal_user
   end
 
-  scenario 'TMI during login', :js => true do
+  scenario 'TMI during login - username' do
     visit '/'
     within('.signup') do
       fill_in 'email', :with => @normal_user.email + 'not'
       fill_in 'password', :with => @normal_user.clear_password
     end
     click_on 'Login'
-    find('div#flash_notice').text.should == "#{@normal_user.email}not doesn't exist!"
+    pending(:if => verifying_fixed?) { find('div#flash_notice').text.should == "#{@normal_user.email}not doesn't exist!" }
+  end
 
+  scenario 'TMI during login - password' do
+    visit '/'
     within('.signup') do
       fill_in 'email', :with => @normal_user.email
       fill_in 'password', :with => @normal_user.clear_password + 'not'
     end
     click_on 'Login'
-    find('div#flash_notice').text.should == 'Incorrect Password!'
+    pending(:if => verifying_fixed?) { find('div#flash_notice').text.should == 'Incorrect Password!' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/command_injection_spec.rb b/spec/features/command_injection_spec.rb
index d2b8a55..468e92d 100644
--- a/spec/features/command_injection_spec.rb
+++ b/spec/features/command_injection_spec.rb
@@ -8,7 +8,7 @@ feature 'command injection' do
   end
 
   scenario 'injection attack on file upload', :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' }
@@ -23,6 +23,6 @@ feature 'command injection' do
       end
       click_on 'Start Upload'
     end
-    File.exists?(legit_file).should be_false
+    pending(:if => verifying_fixed?) { File.exists?(legit_file).should be_false }
   end
 end
\ No newline at end of file
diff --git a/spec/features/csrf_spec.rb b/spec/features/csrf_spec.rb
index b088291..b3e56fb 100644
--- a/spec/features/csrf_spec.rb
+++ b/spec/features/csrf_spec.rb
@@ -39,6 +39,6 @@ feature 'csrf' do
       end
     end
 
-    @normal_user.reload.paid_time_off.schedule.last.event_name.should == 'Bad Guy'
+    pending(:if => verifying_fixed?) { @normal_user.reload.paid_time_off.schedule.last.event_name.should == 'Bad Guy' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/info_disclosure_spec.rb b/spec/features/info_disclosure_spec.rb
index 7ecee45..cc93282 100644
--- a/spec/features/info_disclosure_spec.rb
+++ b/spec/features/info_disclosure_spec.rb
@@ -13,6 +13,6 @@ feature 'sensitive information disclosure' do
     login @normal_user
 
     visit "/users/#{@normal_user.user_id}/work_info"
-    page.source.should include '999-99-9999'
+    pending(:if => verifying_fixed?) { page.source.should include '999-99-9999' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/insecure_dor_spec.rb b/spec/features/insecure_dor_spec.rb
index b0ac570..ce089e2 100644
--- a/spec/features/insecure_dor_spec.rb
+++ b/spec/features/insecure_dor_spec.rb
@@ -13,9 +13,11 @@ feature 'insecure direct object reference' do
     download_url = first('.widget-body a')[:href]
     visit download_url.sub(/name=(.*?)&/, 'name=../../config/database.yml&')
 
-    page.status_code.should == 200
-    page.response_headers['Content-Disposition'].should include('database.yml')
-    page.response_headers['Content-Length'].should == '576'
+    pending(:if => verifying_fixed?) {
+      page.status_code.should == 200
+      page.response_headers['Content-Disposition'].should include('database.yml')
+      page.response_headers['Content-Length'].should == '576'
+    }
   end
 
   scenario 'view any user work_info' do
@@ -24,6 +26,6 @@ feature 'insecure direct object reference' do
     @normal_user.user_id.should_not == 2
     visit '/users/2/work_info'
 
-    first('td').text.should == 'Jack Mannino'
+    pending(:if => verifying_fixed?) { first('td').text.should == 'Jack Mannino' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/mass_assignment_spec.rb b/spec/features/mass_assignment_spec.rb
index a818656..0e89b65 100644
--- a/spec/features/mass_assignment_spec.rb
+++ b/spec/features/mass_assignment_spec.rb
@@ -6,7 +6,7 @@ feature 'sql injection' do
     @normal_user = UserFixture.normal_user
   end
 
-  scenario 'mass assignment attack on account_settings' do
+  scenario 'mass assignment attack update account_settings' do
     @normal_user.admin.should be_false
 
     login(@normal_user)
@@ -17,6 +17,21 @@ feature 'sql injection' do
                         :password_confirmation => @normal_user.clear_password}}
     page.driver.put "/users/#{@normal_user.user_id}.json", params
 
-    @normal_user.reload.admin.should be_true
+    pending(:if => verifying_fixed?) { @normal_user.reload.admin.should be_true }
   end
-end
\ No newline at end of file
+
+  scenario 'mass assignment attack create new account' do
+    params = {:user => {:admin => 't',
+                        :email => 'hackety@h4x0rs.c0m',
+                        :first_name => 'hackety',
+                        :last_name => 'hax',
+                        :password => 'foobarewe',
+                        :password_confirmation => 'foobarewe'}}
+    page.driver.post '/users', params
+
+    pending(:if => verifying_fixed?) {
+      User.last.email.should == 'hackety@h4x0rs.c0m'
+      User.last.admin.should be_true
+    }
+  end
+end
diff --git a/spec/features/sql_injection_spec.rb b/spec/features/sql_injection_spec.rb
index 9553fc4..45a2800 100644
--- a/spec/features/sql_injection_spec.rb
+++ b/spec/features/sql_injection_spec.rb
@@ -23,8 +23,10 @@ feature 'sql injection' do
     end
     click_on 'Submit'
 
-    @admin_user = User.where("admin='t'").first
-    @admin_user.email.should == 'joe.admin@schmoe.com'
-    @admin_user.admin.should == true
+    pending(:if => verifying_fixed?) {
+      @admin_user = User.where("admin='t'").first
+      @admin_user.email.should == 'joe.admin@schmoe.com'
+      @admin_user.admin.should == true
+    }
   end
 end
\ No newline at end of file
diff --git a/spec/features/unvalidated_redirects_spec.rb b/spec/features/unvalidated_redirects_spec.rb
index 8b52f7e..8db28e8 100644
--- a/spec/features/unvalidated_redirects_spec.rb
+++ b/spec/features/unvalidated_redirects_spec.rb
@@ -14,6 +14,6 @@ feature 'unvalidated redirect' do
     end
     click_on 'Login'
 
-    current_url.should == 'http://example.com/do/evil/things'
+    pending(:if => verifying_fixed?) { current_url.should == 'http://example.com/do/evil/things' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/url_access_spec.rb b/spec/features/url_access_spec.rb
index 7e2992d..f33f858 100644
--- a/spec/features/url_access_spec.rb
+++ b/spec/features/url_access_spec.rb
@@ -10,6 +10,6 @@ feature 'url access' do
     login @normal_user
 
     visit '/admin/1/dashboard'
-    current_path.should == '/admin/1/dashboard'
+    pending(:if => verifying_fixed?) { current_path.should == '/admin/1/dashboard' }
   end
 end
\ No newline at end of file
diff --git a/spec/features/xss_spec.rb b/spec/features/xss_spec.rb
index f7c4694..264eaaf 100644
--- a/spec/features/xss_spec.rb
+++ b/spec/features/xss_spec.rb
@@ -21,7 +21,7 @@ feature 'xss' do
 
     visit '/'
 
-    find('form.button_to input.btn.btn-primary').value.should == 'RailsGoat h4x0r3d'
+    pending(:if => verifying_fixed?) { find('form.button_to input.btn.btn-primary').value.should == 'RailsGoat h4x0r3d' }
 
     # might be nice to demonstrate posting cookie contents or somesuch, but
     # this at least shows the vulnerability still exists.
diff --git a/spec/support/capybara_shared.rb b/spec/support/capybara_shared.rb
index aeeb960..55cd9d3 100644
--- a/spec/support/capybara_shared.rb
+++ b/spec/support/capybara_shared.rb
@@ -1,3 +1,14 @@
+# By default this will return true, and thus all of the Capybara specs will
+# fail until a developer using the site for training has patched up all of
+# the vulnerabilities.
+#
+# However, RailsGoat maintainers need the Capybara features to pass to indicate
+# changes to the site have not inadvertently removed or fixed any vulnerabilities
+# since the whole point is to provide a site for a developer to fix.
+def verifying_fixed?
+  !ENV['RAILSGOAT_MAINTAINER']
+end
+
 def login(user)
   visit '/'
   within('.signup') do