Skip to content

Commit

Permalink
Merge pull request from GHSA-qxmr-qxh6-2cc9
Browse files Browse the repository at this point in the history
Fix ReDos vulnerability on Spree::EmailValidator::EMAIL_REGEXP
  • Loading branch information
waiting-for-dev authored Dec 7, 2021
2 parents c3afe4e + d2fc8cf commit 6be174c
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 8 deletions.
2 changes: 1 addition & 1 deletion core/lib/spree/core/validators/email.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ module Spree
# end
#
class EmailValidator < ActiveModel::EachValidator
EMAIL_REGEXP = /\A([^@\.]|[^@\.]([^@\s]*)[^@\.])@([^@\s]+\.)+[^@\s]+\z/
EMAIL_REGEXP = URI::MailTo::EMAIL_REGEXP

def validate_each(record, attribute, value)
unless EMAIL_REGEXP.match? value
Expand Down
18 changes: 18 additions & 0 deletions core/lib/tasks/solidus/check_orders_with_invalid_email.rake
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# frozen_string_literal: true

namespace :solidus do
desc 'Prints orders with invalid email (after fix for GHSA-qxmr-qxh6-2cc9)'
task check_orders_with_invalid_email: :environment do
matches = Spree::Order.find_each.reduce([]) do |matches, order|
order.email.nil? || Spree::EmailValidator::EMAIL_REGEXP.match?(order.email) ? matches : matches + [order]
end
if matches.any?
puts 'Email / ID / Number'
puts(matches.map do |order|
"#{order.email} / #{order.id} / #{order.number}"
end.join("\n"))
else
puts 'NO MATCHES'
end
end
end
11 changes: 4 additions & 7 deletions core/spec/lib/spree/core/validators/email_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,30 +24,27 @@ class Tester
let(:invalid_emails) {
[
'invalid [email protected]',
'[email protected]',
'[email protected]',
'@email.com',
'[email protected]',
'invalidemailemail.com',
'@[email protected]',
'invalid@[email protected]',
'invalid.email@@email.com'
]
}

it 'validates valid email addresses' do
it 'validates valid email addresses', :aggregate_failures do
tester = Tester.new
valid_emails.each do |email|
tester.email_address = email
expect(tester.valid?).to be true
expect(tester.valid?).to be(true), "expected #{email} to be valid"
end
end

it 'validates invalid email addresses' do
it 'validates invalid email addresses', :aggregate_failures do
tester = Tester.new
invalid_emails.each do |email|
tester.email_address = email
expect(tester.valid?).to be false
expect(tester.valid?).to be(false), "expected #{email} not to be valid"
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# frozen_string_literal: true

require 'rails_helper'

path = Spree::Core::Engine.root.join('lib/tasks/solidus/check_orders_with_invalid_email.rake')

RSpec.describe 'solidus' do
describe 'check_orders_with_invalid_email' do
include_context(
'rake',
task_path: path,
task_name: 'solidus:check_orders_with_invalid_email'
)

it 'includes orders with invalid email' do
order = create(:order)
order.update_column(:email, 'invalid [email protected]')

expect { task.invoke }.to output(/invalid [email protected] \/ #{order.id} \/ #{order.number}\n/).to_stdout
end

it "doesn't include orders with valid email" do
order = create(:order, email: '[email protected]')

expect { task.invoke }.not_to output(/[email protected]/).to_stdout
end

it "doesn't include orders with no email" do
order = create(:order, user: nil, email: nil, number: '123')

expect { task.invoke }.not_to output(/#{order.number}/).to_stdout
end

it "prints message when no matches found" do
expect { task.invoke }.to output(/NO MATCHES/).to_stdout
end
end
end

0 comments on commit 6be174c

Please sign in to comment.