From c39b0c35fdd376c70d16973eef62bccd2ed4e09b Mon Sep 17 00:00:00 2001 From: cktricky Date: Tue, 6 Jan 2015 13:14:53 -0500 Subject: [PATCH 1/5] resolves issue #180 --- app/views/layouts/shared/_footer.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/layouts/shared/_footer.html.erb b/app/views/layouts/shared/_footer.html.erb index 1c1a443..4bb925f 100755 --- a/app/views/layouts/shared/_footer.html.erb +++ b/app/views/layouts/shared/_footer.html.erb @@ -1,6 +1,6 @@ From 73e8ab972b29a716e0f87cb053f99b058a01b2bf Mon Sep 17 00:00:00 2001 From: chrismo Date: Tue, 6 Jan 2015 11:47:05 -0600 Subject: [PATCH 2/5] assign_user_id and UserFixture password fixes. When the database is empty, which can happen in the test database and in the dev database if the seeds.rb aren't applied, the assign_user_id method would not assign an id and the newer before_filter block to generate_token would fail. UserFixture had a password on it that wouldn't pass the new validation rules once that vulnerability is patched. --- app/models/user.rb | 6 +++++- spec/support/user_fixture.rb | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9c5cc7f..21b4fd1 100755 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -77,7 +77,11 @@ class User < ActiveRecord::Base def assign_user_id unless @skip_user_id_assign.present? || self.user_id.present? user = User.order("user_id").last - uid = user.user_id.to_i + 1 if user && user.user_id && !(User.exists?(:user_id => "#{user.user_id.to_i + 1}")) + uid = if user && user.user_id && !(User.exists?(:user_id => "#{user.user_id.to_i + 1}")) + user.user_id.to_i + 1 + else + 1 + end self.user_id = uid.to_s if uid end end diff --git a/spec/support/user_fixture.rb b/spec/support/user_fixture.rb index 8a5f182..bb71be2 100644 --- a/spec/support/user_fixture.rb +++ b/spec/support/user_fixture.rb @@ -5,14 +5,14 @@ class UserFixture end def self.normal_user - password = 'aoeuaoeu' + password = 'thi$ 1s cOmplExEr' user = User.new(:first_name => 'Joe', :last_name => 'Schmoe', :email => 'joe@schmoe.com', :password => password, :password_confirmation => password) def user.clear_password - 'aoeuaoeu' + 'thi$ 1s cOmplExEr' end user.build_benefits_data user.save! user end -end \ No newline at end of file +end From e9f66b86946eb6c59a00a912851b6ab5f2bea7cd Mon Sep 17 00:00:00 2001 From: cktricky Date: Tue, 6 Jan 2015 13:44:58 -0500 Subject: [PATCH 3/5] deleted unnecessary file --- report.html | 1606 --------------------------------------------------- 1 file changed, 1606 deletions(-) delete mode 100644 report.html diff --git a/report.html b/report.html deleted file mode 100644 index c8c5512..0000000 --- a/report.html +++ /dev/null @@ -1,1606 +0,0 @@ - - - - -Brakeman Report - - - - - -

Brakeman Report

- - - - - - - - - - - - - - -
Application PathRails VersionBrakeman VersionReport TimeChecks Performed
/Users/cktricky/tmp/railsgoat3.2.112.6.1 - - 2014-07-29 12:41:05 -0500

- 2.412842 seconds - 		BasicAuth, ContentTag, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, EscapeFunction, Evaluation, Execute, FileAccess, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NumberToCurrency, QuoteTableName, Redirect, RegexDoS, Render, RenderDoS, ResponseSplitting, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, StripTags, SymbolDoS, TranslateBug, UnsafeReflection, ValidationRegex, WithoutProtection, YAMLParsing
-
-

Summary

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Scanned/ReportedTotal
Controllers17
Models11
Templates73
Errors0
Security Warnings27 (16)
Ignored Warnings0
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Warning TypeTotal
Attribute Restriction1
Command Injection1
Cross Site Scripting5
Cross-Site Request Forgery1
Denial of Service2
File Access1
Format Validation1
Mass Assignment5
Remote Code Execution5
SQL Injection3
Session Setting2
-
-

Security Warnings

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
ConfidenceClassMethodWarning TypeMessage
HighBenefitFormsControllerdownloadFile Access
Parameter value used in file name near line 11: send_file(params[:type].constantize.new(params[:name]... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighApi::V1::MobileControllershowRemote Code Execution
Unsafe reflection method constantize called with parameter value near line 9: params[:class].classify... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighApi::V1::MobileControllerindexRemote Code Execution
Unsafe reflection method constantize called with parameter value near line 16: params[:class].classif... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighBenefitFormsControllerdownloadRemote Code Execution
Unsafe reflection method constantize called with parameter value near line 10: params[:type].constant... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighSession Setting
Session cookies should be set to HTTP only near line 3 - - - - - - - - - - - - - - - - - - - - - - - -
HighSession Setting
Session secret should not be included in version control near line 7 - - - - - - - - - - - - - - - - - - - -
HighUsersControllerupdateSQL Injection
Possible SQL injection near line 34: User.find(:first, :conditions => ("user_id = '#{params[:user][:u... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighSQL InjectionRails 3.2.11 contains a SQL injection vulnerability (CVE-2013-6417). Upgrade to 3.2.16
MediumBenefitsBenefits.make_backupCommand Injection
Possible command injection near line 15: system("cp #{(local full_file_name)} #{(local data_path)}/ba... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
MediumDenial of ServiceRails 3.2.11 has a denial of service vulnerability in ActiveRecord: upgrade to 3.2.13 or patch
MediumRemote Code Execution
Rails 3.2.11 with globbing routes is vulnerable to directory traversal and remote code execution. Pat...
MediumAnalyticshits_by_ipSQL Injection
Possible SQL injection near line 4: select("#{(local col)}") - - - - - - - - - - - - - - - - - - - - - - - -
MediumPasswordResetsControllerreset_passwordRemote Code Execution
Marshal.load called with parameter value near line 5: Marshal.load(Base64.decode64(params[:user])) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
MediumCross Site ScriptingRails 3.2.11 has a vulnerability in number helpers (CVE-2014-0081). Upgrade to Rails version 3.2.17
MediumDenial of ServiceRails 3.2.11 has a denial of service vulnerability (CVE-2013-6414). Upgrade to Rails version 3.2.16
-

Controller Warnings

- - - - - - - - - - - - - - - -
ConfidenceControllerWarning TypeMessage
HighApplicationControllerCross-Site Request Forgery'protect_from_forgery' should be called in ApplicationController

Model Warnings

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
ConfidenceModelWarning TypeMessage
HighBenefitsAttribute RestrictionMass assignment is not restricted using attr_accessible
HighUserFormat Validation
Insufficient validation for 'email' using /.+@.+\..+/i. Use \A and \z as anchors near line 12 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighUserMass AssignmentPotentially dangerous attribute available for mass assignment: :admin
WeakKeyManagementMass AssignmentPotentially dangerous attribute available for mass assignment: :user_id
WeakMessageMass AssignmentPotentially dangerous attribute available for mass assignment: :creator_id
WeakMessageMass AssignmentPotentially dangerous attribute available for mass assignment: :receiver_id
WeakUserMass AssignmentPotentially dangerous attribute available for mass assignment: :user_id

View Warnings

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
ConfidenceTemplateWarning TypeMessage
High - - layouts/application (AdminController#dashboard) - - Cross Site Scripting
Unescaped cookie value near line 12: cookies[:font] - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
High - - pay/index (PayController#index) - - Cross Site Scripting
Rails 3.2.11 has a vulnerability in sanitize: upgrade to 3.2.13 or patch near line 188: sanitize(user... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
High - - pay/index (PayController#index) - - Cross Site Scripting
Rails 3.2.11 has a vulnerability in sanitize: upgrade to 3.2.13 or patch near line 239: sanitize(decr... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
High - - pay/index (PayController#index) - - Cross Site Scripting
Rails 3.2.11 has a vulnerability in sanitize: upgrade to 3.2.13 or patch near line 261: sanitize(upda... - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
- From 907045488d404bb6f608b1757279fef31e9485c6 Mon Sep 17 00:00:00 2001 From: cktricky Date: Fri, 9 Jan 2015 11:40:37 -0500 Subject: [PATCH 4/5] this change allows the app to get the csrf fixes working when running rake training --- config/environments/test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/environments/test.rb b/config/environments/test.rb index 71d265d..d842cdd 100755 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -22,7 +22,7 @@ Railsgoat::Application.configure do config.action_dispatch.show_exceptions = false # Disable request forgery protection in test environment - config.action_controller.allow_forgery_protection = false + config.action_controller.allow_forgery_protection = true # Tell Action Mailer not to deliver emails to the real world. # The :test delivery method accumulates sent emails in the From 3d29293bd46b0c3a27b98df0f034f925c3ff8c5f Mon Sep 17 00:00:00 2001 From: cktricky Date: Sun, 8 Feb 2015 18:10:27 -0500 Subject: [PATCH 5/5] pry instead of rails c --- Gemfile | 3 +++ Gemfile.lock | 3 +++ 2 files changed, 6 insertions(+) diff --git a/Gemfile b/Gemfile index 3539b41..f00a614 100755 --- a/Gemfile +++ b/Gemfile @@ -11,6 +11,9 @@ ruby '2.1.5' gem 'sqlite3' gem 'foreman' +# Pry for Rails, not in dev group in case running via prod/staging @ a training +gem 'pry-rails' + group :development, :mysql do gem 'brakeman' gem 'bundler-audit' diff --git a/Gemfile.lock b/Gemfile.lock index 5516b7c..4034036 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -184,6 +184,8 @@ GEM coderay (~> 1.1.0) method_source (~> 0.8.1) slop (~> 3.4) + pry-rails (0.3.3) + pry (>= 0.9.10) rack (1.4.5) rack-cache (1.2) rack (>= 0.4) @@ -328,6 +330,7 @@ DEPENDENCIES poltergeist powder pry + pry-rails rack-livereload rails (= 3.2.21) rb-fsevent