From 22c39a15818302a52930cb9ac421b8dc3838d341 Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Thu, 7 Sep 2023 10:30:54 -0400 Subject: [PATCH 1/2] fixed format-validation vulnerability for submitting phone number --- Gemfile | 3 +- Gemfile.lock | 2 + app/models/submitter.rb | 2 +- brakeman.html | 643 ++++++++++++++++++++++++++++++++++ spec/models/submitter_spec.rb | 24 +- 5 files changed, 667 insertions(+), 7 deletions(-) create mode 100644 brakeman.html diff --git a/Gemfile b/Gemfile index 15302537..f5fb5506 100644 --- a/Gemfile +++ b/Gemfile @@ -65,6 +65,7 @@ group :development, :test do end group :development do + gem 'brakeman', '~> 6.0' gem 'capistrano', '~> 3.17.1', require: false gem 'capistrano-bundler', '~> 1.6', require: false gem 'capistrano-rails', '~> 1.4', require: false @@ -73,10 +74,10 @@ group :development do gem 'capistrano-rvm', require: false # Access an interactive console on exception pages or by calling 'console' anywhere in the code. gem 'listen', '>= 3.0.5', '< 3.2' - gem 'web-console', '>= 3.3.0' # Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring gem 'spring' gem 'spring-watcher-listen', '~> 2.0.0' + gem 'web-console', '>= 3.3.0' end group :test do diff --git a/Gemfile.lock b/Gemfile.lock index 13ed3b29..bdf2f49e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -76,6 +76,7 @@ GEM autoprefixer-rails (>= 9.1.0) popper_js (>= 1.14.3, < 2) sassc-rails (>= 2.0.0) + brakeman (6.0.1) builder (3.2.4) byebug (11.1.3) capistrano (3.17.3) @@ -372,6 +373,7 @@ DEPENDENCIES bcrypt_pbkdf bootsnap (>= 1.1.0) bootstrap (~> 4.4.1) + brakeman (~> 6.0) byebug capistrano (~> 3.17.1) capistrano-bundler (~> 1.6) diff --git a/app/models/submitter.rb b/app/models/submitter.rb index 3022c306..6c5584e0 100644 --- a/app/models/submitter.rb +++ b/app/models/submitter.rb @@ -5,7 +5,7 @@ class Submitter < ApplicationRecord validates :first_name, presence: true validates :last_name, presence: true validates :mailing_address, presence: true - validates :phone_number, presence: true, format: { with: /\d{3}-\d{3}-\d{4}/, message: 'Please use the format 111-111-1111' } + validates :phone_number, presence: true, format: { with: /\A\d{3}-\d{3}-\d{4}\z/, message: 'Please use the format 111-111-1111' } validates :email_address, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP, message: 'Please enter a valid email' } def self.to_csv diff --git a/brakeman.html b/brakeman.html new file mode 100644 index 00000000..a621d503 --- /dev/null +++ b/brakeman.html @@ -0,0 +1,643 @@ + + + + +Brakeman Report + + + + + + + +

Brakeman Report

+ + + + + + + + + + + + + + + + + + +
Application PathRails VersionBrakeman VersionReport TimeChecks Performed
/Users/huyckjl/Codebases/aaec6.1.7.66.0.1 + + 2023-09-07 10:18:36 -0400

+ 0.874537 seconds + 		BasicAuth, BasicAuthTimingAttack, CSRFTokenForgeryCVE, ContentTag, CookieSerialization, CreateWith, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, DynamicFinders, EOLRails, EOLRuby, EscapeFunction, Evaluation, Execute, FileAccess, FileDisclosure, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONEncoding, JSONEntityEscape, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, MimeTypeDoS, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NestedAttributesBypass, NumberToCurrency, PageCachingCVE, Pathname, PermitAttributes, QuoteTableName, Redirect, RegexDoS, Render, RenderDoS, RenderInline, ResponseSplitting, RouteDoS, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeConfigCve, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionManipulation, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, SprocketsPathTraversal, StripTags, SymbolDoSCVE, TemplateInjection, TranslateBug, UnsafeReflection, UnsafeReflectionMethods, ValidationRegex, VerbConfusion, WeakRSAKey, WithoutProtection, XMLDoS, YAMLParsing
+
+

Summary

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Scanned/ReportedTotal
Controllers19
Models16
Templates89
Errors0
Security Warnings4 (3)
Ignored Warnings0
+
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
Warning TypeTotal
Dynamic Render Path1
File Access1
Format Validation1
Remote Code Execution1
+
+

Security Warnings

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
ConfidenceClassMethodWarning TypeCWE IDMessage
HighPagesControllervalid_page?File Access[22]
Parameter value used in file name near line 17: Pathname.new((Rails.root + "app/views/pages/#{params[:page]}.html.erb")) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
HighAdminControllercsvRemote Code Execution[470]
Unsafe reflection method const_get called with parameter value near line 18: Object.const_get(params[:controller_name].classify) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
MediumPagesControllershowDynamic Render Path[22]
Render path contains parameter value near line 8: render(template => "pages/#{params[:page]}", {}) + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+

Model Warnings

+ + + + + + + + + + + + + + + + + + + + + +
ConfidenceModelWarning TypeCWE IDMessage
HighSubmitterFormat Validation[777]
Insufficient validation for phone_number using /\d{3}-\d{3}-\d{4}/. Use \A and \z as anchors near line 8 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
\ No newline at end of file diff --git a/spec/models/submitter_spec.rb b/spec/models/submitter_spec.rb index 8e577f84..12473b59 100644 --- a/spec/models/submitter_spec.rb +++ b/spec/models/submitter_spec.rb @@ -26,18 +26,32 @@ expect(subject).to_not be_valid end + it 'is valid with a properly formatted phone_number' do + subject.phone_number = '111-111-1111' + expect(subject).to be_valid + end + it 'is not valid without a phone_number' do subject.phone_number = nil expect(subject).to_not be_valid end - it 'is not valid without a email_address' do - subject.email_address = nil - expect(subject).to_not be_valid + it 'is not valid with an improperly formatted phone_number' do + [ + '1111111111', # no dashes + '111-1111-1111', # too many digits + '11-111-1111', # too few digits + '111-111-1111abc', # additional characters + 'abc111-111-1111', # additional characters + '1-111-111-1111' # too many sections and digits + ].each do |invalid_number| + subject.phone_number = invalid_number + expect(subject).to_not be_valid, "Expected #{invalid_number} to be invalid" + end end - it 'is not valid without a formatted phone_number' do - subject.phone_number = '1111111111' + it 'is not valid without a email_address' do + subject.email_address = nil expect(subject).to_not be_valid end From 6b385b674306a9fa226a3ba728cc5a48c92d2776 Mon Sep 17 00:00:00 2001 From: Janell-Huyck Date: Thu, 7 Sep 2023 10:32:19 -0400 Subject: [PATCH 2/2] remove brakeman file --- brakeman.html | 643 -------------------------------------------------- 1 file changed, 643 deletions(-) delete mode 100644 brakeman.html diff --git a/brakeman.html b/brakeman.html deleted file mode 100644 index a621d503..00000000 --- a/brakeman.html +++ /dev/null @@ -1,643 +0,0 @@ - - - - -Brakeman Report - - - - - - - -

Brakeman Report

- - - - - - - - - - - - - - - - - - -
Application PathRails VersionBrakeman VersionReport TimeChecks Performed
/Users/huyckjl/Codebases/aaec6.1.7.66.0.1 - - 2023-09-07 10:18:36 -0400

- 0.874537 seconds - 		BasicAuth, BasicAuthTimingAttack, CSRFTokenForgeryCVE, ContentTag, CookieSerialization, CreateWith, CrossSiteScripting, DefaultRoutes, Deserialize, DetailedExceptions, DigestDoS, DynamicFinders, EOLRails, EOLRuby, EscapeFunction, Evaluation, Execute, FileAccess, FileDisclosure, FilterSkipping, ForgerySetting, HeaderDoS, I18nXSS, JRubyXML, JSONEncoding, JSONEntityEscape, JSONParsing, LinkTo, LinkToHref, MailTo, MassAssignment, MimeTypeDoS, ModelAttrAccessible, ModelAttributes, ModelSerialize, NestedAttributes, NestedAttributesBypass, NumberToCurrency, PageCachingCVE, Pathname, PermitAttributes, QuoteTableName, Redirect, RegexDoS, Render, RenderDoS, RenderInline, ResponseSplitting, RouteDoS, SQL, SQLCVEs, SSLVerify, SafeBufferManipulation, SanitizeConfigCve, SanitizeMethods, SelectTag, SelectVulnerability, Send, SendFile, SessionManipulation, SessionSettings, SimpleFormat, SingleQuotes, SkipBeforeFilter, SprocketsPathTraversal, StripTags, SymbolDoSCVE, TemplateInjection, TranslateBug, UnsafeReflection, UnsafeReflectionMethods, ValidationRegex, VerbConfusion, WeakRSAKey, WithoutProtection, XMLDoS, YAMLParsing
-
-

Summary

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Scanned/ReportedTotal
Controllers19
Models16
Templates89
Errors0
Security Warnings4 (3)
Ignored Warnings0
-
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Warning TypeTotal
Dynamic Render Path1
File Access1
Format Validation1
Remote Code Execution1
-
-

Security Warnings

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
ConfidenceClassMethodWarning TypeCWE IDMessage
HighPagesControllervalid_page?File Access[22]
Parameter value used in file name near line 17: Pathname.new((Rails.root + "app/views/pages/#{params[:page]}.html.erb")) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
HighAdminControllercsvRemote Code Execution[470]
Unsafe reflection method const_get called with parameter value near line 18: Object.const_get(params[:controller_name].classify) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
MediumPagesControllershowDynamic Render Path[22]
Render path contains parameter value near line 8: render(template => "pages/#{params[:page]}", {}) - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-

Model Warnings

- - - - - - - - - - - - - - - - - - - - - -
ConfidenceModelWarning TypeCWE IDMessage
HighSubmitterFormat Validation[777]
Insufficient validation for phone_number using /\d{3}-\d{3}-\d{4}/. Use \A and \z as anchors near line 8 - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
\ No newline at end of file