diff --git a/.gitignore b/.gitignore index fef3086..c58b054 100755 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,5 @@ .elasticbeanstalk/ .DS_Store /public/data -*.png \ No newline at end of file +*.png +coverage \ No newline at end of file diff --git a/Gemfile b/Gemfile index c3836db..131581f 100755 --- a/Gemfile +++ b/Gemfile @@ -26,6 +26,8 @@ end gem 'gauntlt' +gem 'simplecov', '0.8.0.pre2', :require => false, :group => :test + group :development, :test do gem 'launchy' gem 'capybara' diff --git a/Gemfile.lock b/Gemfile.lock index 92d0f04..aed7bd1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -40,11 +40,11 @@ GEM erubis (>= 2.6.6) binding_of_caller (0.7.2) debug_inspector (>= 0.0.1) - brakeman (2.1.2) + brakeman (2.2.0) erubis (~> 2.6) fastercsv (~> 1.5) haml (>= 3.0, < 5.0) - highline (~> 1.6.19) + highline (~> 1.6.20) multi_json (~> 1.2) ruby2ruby (~> 2.0.5) ruby_parser (~> 3.2.2) @@ -82,6 +82,7 @@ GEM database_cleaner (1.0.1) debug_inspector (0.0.2) diff-lcs (1.2.4) + docile (1.1.0) dotenv (0.9.0) em-websocket (0.5.0) eventmachine (>= 0.12.9) @@ -135,10 +136,11 @@ GEM launchy (2.3.0) addressable (~> 2.3) libv8 (3.16.14.3) - listen (2.1.1) + listen (2.1.2) celluloid (>= 0.15.2) rb-fsevent (>= 0.9.3) rb-inotify (>= 0.9) + lockfile (2.1.0) mail (2.5.4) mime-types (~> 1.16) treetop (~> 1.4.8) @@ -218,7 +220,13 @@ GEM sass (>= 3.1.10) tilt (~> 1.3) sexp_processor (4.4.0) - slim (2.0.1) + simplecov (0.8.0.pre2) + docile (~> 1.1.0) + lockfile (>= 2.1.0) + multi_json + simplecov-html (~> 0.7.1) + simplecov-html (0.7.1) + slim (2.0.2) temple (~> 0.6.6) tilt (>= 1.3.3, < 2.1) slop (2.1.0) @@ -243,9 +251,9 @@ GEM polyglot (>= 0.3.1) trollop (2.0) tzinfo (0.3.38) - uglifier (2.2.1) + uglifier (2.3.0) execjs (>= 0.3.0) - multi_json (~> 1.0, >= 1.0.2) + json (>= 1.8.0) unicorn (4.6.3) kgio (~> 2.6) rack @@ -285,6 +293,7 @@ DEPENDENCIES rb-fsevent rspec-rails sass-rails + simplecov (= 0.8.0.pre2) sqlite3 therubyracer travis-lint diff --git a/README.md b/README.md index 850287c..10fe978 100755 --- a/README.md +++ b/README.md @@ -49,7 +49,7 @@ Then proceed with browsing the site as normal :thumbsup: [](https://codeclimate.com/github/OWASP/railsgoat) -[](https://travis-ci.org/mccabe615/railsgoat) +[](https://travis-ci.org/OWASP/railsgoat) ### License Stuff ### diff --git a/app/models/benefits.rb b/app/models/benefits.rb index 985b8cc..44a467d 100644 --- a/app/models/benefits.rb +++ b/app/models/benefits.rb @@ -11,15 +11,28 @@ class Benefits < ActiveRecord::Base end def self.make_backup(file, data_path, full_file_name) - if File.exists?(full_file_name) - system("cp #{full_file_name} #{data_path}/bak#{Time.now.to_i}_#{file.original_filename}") - end - end + if File.exists?(full_file_name) + silence_streams(STDERR) { system("cp #{full_file_name} #{data_path}/bak#{Time.now.to_i}_#{file.original_filename}") } + end + end =begin def self.make_backup(file, data_path, full_file_name) FileUtils.cp "#{full_file_name}", "#{data_path}/bak#{Time.now.to_i}_#{file.original_filename}" end =end + + def self.silence_streams(*streams) + on_hold = streams.collect { |stream| stream.dup } + streams.each do |stream| + stream.reopen(RUBY_PLATFORM =~ /mswin/ ? 'NUL:' : '/dev/null') + stream.sync = true + end + yield + ensure + streams.each_with_index do |stream, i| + stream.reopen(on_hold[i]) + end + end end diff --git a/app/views/layouts/tutorial/csrf/_csrf_first.html.erb b/app/views/layouts/tutorial/csrf/_csrf_first.html.erb index ff4f512..1ae7ecd 100755 --- a/app/views/layouts/tutorial/csrf/_csrf_first.html.erb +++ b/app/views/layouts/tutorial/csrf/_csrf_first.html.erb @@ -60,7 +60,7 @@
Cross-Site Request Forgery ATTACK:
- The application allows users to update their calendar and schedule PTO events (PTO section). Due to the fact CSRF protections are disabled, the AJAX request will send the authenticity token but the application will not validate either it's presence or validity. Create an html page using the code shown below, authenticate as another user, click on it, review the new calendar (change the dates under date_range1). You should see this HTML code will work, even if you hadn't navigated to the PTO section prior to sending it. + The application allows users to update their calendar and schedule PTO events (PTO section). Due to the fact CSRF protections are disabled, the AJAX request will send the authenticity token but the application will not validate either its presence or validity. Create an html page using the code shown below, authenticate as another user, click on it, review the new calendar (change the dates under date_range1). You should see this HTML code will work, even if you hadn't navigated to the PTO section prior to sending it.
@@ -84,7 +84,7 @@Cross-Site Request Forgery SOLUTION:
- By Default, the protect_from_forgery directive is added under the application_controller.rb at project creation. However, occasionally developers turn it off (comment out) because of issues with JS. There are two separate solutions around the JS problem. + By default, the protect_from_forgery directive is added under the application_controller.rb at project creation. However, occasionally developers turn it off (comment out) because of issues with JS. There are two separate solutions around the JS problem.
Once protect_from_forgery is added back... diff --git a/app/views/layouts/tutorial/redirects/_redirects_first.html.erb b/app/views/layouts/tutorial/redirects/_redirects_first.html.erb index 10f875f..44aeefb 100755 --- a/app/views/layouts/tutorial/redirects/_redirects_first.html.erb +++ b/app/views/layouts/tutorial/redirects/_redirects_first.html.erb @@ -17,7 +17,8 @@
diff --git a/spec/controllers/messages_controller_spec.rb b/spec/controllers/messages_controller_spec.rb index 503cc98..335cafc 100644 --- a/spec/controllers/messages_controller_spec.rb +++ b/spec/controllers/messages_controller_spec.rb @@ -1,5 +1 @@ -require 'spec_helper' - -describe MessagesController do - -end +require 'spec_helper' \ No newline at end of file diff --git a/spec/helpers/messages_helper_spec.rb b/spec/helpers/messages_helper_spec.rb index a29b665..f8ec369 100644 --- a/spec/helpers/messages_helper_spec.rb +++ b/spec/helpers/messages_helper_spec.rb @@ -1,15 +1 @@ require 'spec_helper' - -# Specs in this file have access to a helper object that includes -# the MessagesHelper. For example: -# -# describe MessagesHelper do -# describe "string concat" do -# it "concats two strings with spaces" do -# expect(helper.concat_strings("this","that")).to eq("this that") -# end -# end -# end -describe MessagesHelper do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/models/message_spec.rb b/spec/models/message_spec.rb index a5f59dd..f8ec369 100644 --- a/spec/models/message_spec.rb +++ b/spec/models/message_spec.rb @@ -1,5 +1 @@ require 'spec_helper' - -describe Message do - pending "add some examples to (or delete) #{__FILE__}" -end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 417153f..e025086 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,10 @@ # This file is copied to spec/ when you run 'rails generate rspec:install' ENV["RAILS_ENV"] ||= 'test' + +# To use simplecov, do this: COVERAGE=true rake +require 'simplecov' +SimpleCov.start if ENV["COVERAGE"] + require File.expand_path("../../config/environment", __FILE__) require 'rspec/rails' require 'rspec/autorun' @@ -50,4 +55,4 @@ end Capybara.javascript_driver = :poltergeist -DatabaseCleaner.strategy = :truncation \ No newline at end of file +DatabaseCleaner.strategy = :truncation diff --git a/spec/support/capybara_shared.rb b/spec/support/capybara_shared.rb index 2f982f9..8606ce4 100644 --- a/spec/support/capybara_shared.rb +++ b/spec/support/capybara_shared.rb @@ -5,32 +5,32 @@ # 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 +$displayed_spec_notice = false def verifying_fixed? maintainer_env_name = 'RAILSGOAT_MAINTAINER' result = !ENV[maintainer_env_name] - if !@@displayed_spec_notice && result + 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. + ****************************************************************************** + 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. + 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 + **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 + $displayed_spec_notice = true end result end @@ -43,3 +43,41 @@ def login(user) end click_on 'Login' end + +##Hack to fix PhantomJS errors on Mavericks - https://gist.github.com/ericboehs/7125105 +module Capybara::Poltergeist + class Client + private + def redirect_stdout + prev = STDOUT.dup + prev.autoclose = false + $stdout = @write_io + STDOUT.reopen(@write_io) + + prev = STDERR.dup + prev.autoclose = false + $stderr = @write_io + STDERR.reopen(@write_io) + yield + ensure + STDOUT.reopen(prev) + $stdout = STDOUT + STDERR.reopen(prev) + $stderr = STDERR + end + end +end + +class WarningSuppressor + class << self + def write(message) + if message =~ /QFont::setPixelSize: Pixel size <= 0/ || message =~/CoreText performance note:/ || message =~/Method userSpaceScaleFactor in class NSView/ then 0 else puts(message);1;end + end + end +end + +Capybara.register_driver :poltergeist do |app| + Capybara::Poltergeist::Driver.new(app, phantomjs_logger: WarningSuppressor) +end + +Capybara.javascript_driver = :poltergeist diff --git a/spec/vulnerabilities/command_injection_spec.rb b/spec/vulnerabilities/command_injection_spec.rb index 9b4ad85..8baed81 100644 --- a/spec/vulnerabilities/command_injection_spec.rb +++ b/spec/vulnerabilities/command_injection_spec.rb @@ -15,7 +15,7 @@ feature 'command injection' do visit "/users/#{@normal_user.user_id}/benefit_forms" Dir.mktmpdir do |dir| - hackety_file = File.join(dir, '; cd public && cd data && rm -f * ;') + 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 diff --git a/spec/vulnerabilities/password_hashing_spec.rb b/spec/vulnerabilities/password_hashing_spec.rb index 077a352..8f3bb02 100644 --- a/spec/vulnerabilities/password_hashing_spec.rb +++ b/spec/vulnerabilities/password_hashing_spec.rb @@ -14,6 +14,7 @@ feature 'improper password hashing' do pending(:if => verifying_fixed?) {Digest::MD5.hexdigest(new_pass).should == @normal_user.password} end +=begin scenario 'with md5 and salt' do pending unless @normal_user.has_attribute?('salt') new_pass = 'testpassword' @@ -22,4 +23,6 @@ feature 'improper password hashing' do @normal_user.save pending(:if => verifying_fixed?) {Digest::MD5.hexdigest(@normal_user.salt + new_pass).should == @normal_user.password} end +=end + end \ No newline at end of file diff --git a/test/test_helper.rb b/test/test_helper.rb index 8bf1192..b757019 100755 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,4 +1,9 @@ ENV["RAILS_ENV"] = "test" + +# To use simplecov, do this: COVERAGE=true rake +require 'simplecov' +SimpleCov.start if ENV["COVERAGE"] + require File.expand_path('../../config/environment', __FILE__) require 'rails/test_help'- OWASP Description - Web applications frequently redirect and forward users to other pages and websites, and use untrusted data to determine the destination pages. Without proper validation, attackers can redirect victims to phishing or malware sites, or use forwards to access unauthorized pages. + Applications frequently redirect users to other pages, or use internal forwards in a similar manner. Sometimes the target page is specified in an unvalidated parameter, allowing attackers to choose the destination page. + Detecting unchecked redirects is easy. Look for redirects where you can set the full URL. Unchecked forwards are harder, because they target internal pages.
Railsgoat allows the redirection to the paths previously requested but for which the user did not have access. Following authentication, the user is redirected. diff --git a/app/views/layouts/tutorial/xss/_xss_first.html.erb b/app/views/layouts/tutorial/xss/_xss_first.html.erb index 4df444d..dc6e516 100755 --- a/app/views/layouts/tutorial/xss/_xss_first.html.erb +++ b/app/views/layouts/tutorial/xss/_xss_first.html.erb @@ -84,7 +84,7 @@
Apparently we had some issues rendering people's names with weird formatting or something, I dunno, I think I fixed it by safely encoding html and rendering the necessary content.
- Your Welcome! + You're Welcome!