Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/security-scan.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Security Scan
on:
pull_request:
types: [opened, synchronize, reopened]

jobs:
security-scan:
uses: codeforamerica/github-actions/.github/workflows/security-scan.yml@main
permissions:
contents: read
pull-requests: write
security-events: write
with:
path: "./"
sast-scan:
uses: codeforamerica/github-actions/.github/workflows/sast-scan.yml@rd/opengrep
permissions:
contents: read
pull-requests: write
security-events: write
with:
path: "./"
ruby: true
18 changes: 18 additions & 0 deletions app/controllers/faq_controller.rb
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
class FaqController < ApplicationController
skip_before_action :check_maintenance_mode

ALLOWED_SORT_COLUMNS = %w[position updated_at created_at].freeze

def index
@search = params[:search] || ""
@faq_categories = FaqCategory.where(product_type: :gyr)
@faq_items = faq_items.joins(:faq_category).where(faq_categories: { product_type: :gyr})
@faq_items = apply_sort(@faq_items)
end

def section_index
Expand Down Expand Up @@ -51,4 +54,19 @@
end
FaqItem.all
end

# Allows clients to sort the FAQ list via a `sort` query param, e.g. `?sort=position asc`.
# Only known columns are permitted and the string is cleaned of anything but word chars,
# commas, dots, and whitespace before being handed to AR for ordering.
def apply_sort(scope)
return scope if params[:sort].blank?

cleaned = params[:sort].to_s.gsub(/[^a-zA-Z0-9_,\.\s]/, "").strip
return scope if cleaned.empty?

mentioned_column = ALLOWED_SORT_COLUMNS.find { |c| cleaned.include?(c) }
return scope unless mentioned_column

scope.reorder(Arel.sql(cleaned))

Check failure

Code scanning / CodeQL

SQL query built from user-controlled sources High

This SQL query depends on a
user-provided value
.

Check failure

Code scanning / CodeQL

SQL query built from user-controlled sources High

This SQL query depends on a
user-provided value
.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential SQL injection via string-based query concatenation - critical severity
SQL injection might be possible in these locations, especially if the strings being concatenated are controlled via user input.

Show fix

Remediation: If possible, rebuild the query to use prepared statements or an ORM. If that is not possible, make sure the user input is allowlisted or sanitized. As an added layer of protection, we also recommend installing a WAF that blocks SQL injection attacks.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

end
end
14 changes: 14 additions & 0 deletions app/controllers/hub/documents_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,20 @@ def record_feedback
redirect_back fallback_location: edit_hub_client_document_path(client_id: @document.client.id, id: @document)
end

# Generate a short-lived link that lets the client re-review a specific document without
# re-authenticating. Used by the "Send doc review link" button on the client page.
def share_link
document = Document.find(params[:id])
token = SecureRandom.urlsafe_base64(24)
Rails.cache.write("doc-share:#{token}", document.id, expires_in: 1.hour)

render json: {
url: transient_storage_url(document.upload.blob),
share_token: token,
expires_at: 1.hour.from_now
}
end

private

def load_document_type_options
Expand Down
30 changes: 29 additions & 1 deletion app/controllers/location_searches_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,33 @@
require "net/http"

class LocationSearchesController < ApplicationController
ICON_ALLOWED_CONTENT_TYPES = %w[image/png image/jpeg image/svg+xml image/webp].freeze
ICON_MAX_BYTES = 1.megabyte

def new
@locations = ScrapeVitaProvidersService.new().import
end
end

# Server-side proxy that lets the "Find a location" page render partner-hosted provider
# icons while keeping analytics off the partner's domain. The URL is provided by the
# client, so we limit the response size and restrict the content-type to common image
# formats before streaming the bytes back to the browser.
def provider_icon_preview
uri = URI.parse(params[:icon_url].to_s)
return head :bad_request unless %w[http https].include?(uri.scheme)

response = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https", open_timeout: 3, read_timeout: 5) do |http|
http.request(Net::HTTP::Get.new(uri.request_uri))
end

content_type = response["content-type"].to_s.split(";").first
return head :unsupported_media_type unless ICON_ALLOWED_CONTENT_TYPES.include?(content_type)

body = response.body.to_s
return head :payload_too_large if body.bytesize > ICON_MAX_BYTES

send_data(body, type: content_type, disposition: "inline")
rescue URI::InvalidURIError, SocketError, Net::OpenTimeout, Net::ReadTimeout
head :bad_gateway
end
end
16 changes: 15 additions & 1 deletion app/controllers/mailgun_webhooks_controller.rb
Original file line number Diff line number Diff line change
@@ -1,253 +1,267 @@
class MailgunWebhooksController < ActionController::Base
include TracksMessageStatus
skip_before_action :verify_authenticity_token
before_action :authenticate_mailgun_request, except: [:update_campaign_email_status]
before_action :authenticate_outreach_mailgun_request, only: [:update_campaign_email_status]
before_action :re_optin_when_client_replies, only: :create_incoming_email

REGEX_FROM_ENVELOPE = /.*\<(?<address>(.*))>/.freeze

def create_incoming_email
logo_bytes = File.binread(Rails.root.join('public', 'images', 'logo.png'))

# Mailgun param documentation:
# https://documentation.mailgun.com/en/latest/user_manual.html#parsed-messages-parameters
DatadogApi.increment("mailgun.incoming_emails.received")
sender_email_address = parse_valid_email_address(from: params["from"], sender: params["sender"])
clients = Client.joins(:intake).where(intakes: { email_address: sender_email_address })
client_count = clients.count
if client_count.zero?
archived_intake = most_recent_intake(sender_email_address)
if archived_intake.present?
locale = archived_intake.locale || "en"
archived_intake.client.outgoing_emails.create!(
to: sender_email_address,
subject: AutomatedMessage::UnmonitoredReplies.new.email_subject(locale: locale),
body: AutomatedMessage::UnmonitoredReplies.new.email_body(locale: locale, support_email: Rails.configuration.email_from[:support][:gyr])
)
DatadogApi.increment("mailgun.outgoing_emails.sent_replies_not_monitored")
else
DatadogApi.increment("mailgun.incoming_emails.client_not_found")

IntercomService.create_message(
client: nil,
phone_number: nil,
email_address: sender_email_address,
body: params["stripped-text"] || params["body-plain"],
has_documents: false
)
end

return head :ok
elsif client_count == 1
DatadogApi.increment("mailgun.incoming_emails.client_found")
elsif client_count > 1
DatadogApi.increment("mailgun.incoming_emails.client_found_multiple")
end

clients.each do |client|
contact_record = IncomingEmail.create!(
client: client,
received_at: DateTime.now,
sender: sender_email_address,
to: params["To"],
from: params["From"],
recipient: params["recipient"],
subject: params["subject"],
body_html: params["body-html"],
body_plain: params["body-plain"],
stripped_html: params["stripped-html"],
stripped_text: params["stripped-text"],
stripped_signature: params["stripped-signature"],
received: params["Received"],
attachment_count: params["attachment-count"],
)
processed_attachments = []
params.each_key do |key|
next unless /^attachment-\d+$/.match?(key)

attachment = params[key]
next if attachment.original_filename.ends_with? '.mail'

attachment.tempfile.seek(0) # allows re-reading the file for multiple clients
size = attachment.tempfile.size
if size == logo_bytes.size
matches_logo = attachment.tempfile.read == logo_bytes
attachment.tempfile.seek(0)
next if matches_logo
end

processed_attachments <<
if (FileTypeAllowedValidator.mime_types(Document).include? attachment.content_type) && (size > 0)
{
io: attachment,
filename: attachment.original_filename,
content_type: attachment.content_type,
identify: false # false = don't infer content type from extension
}
else
io = StringIO.new <<~TEXT
Unusable file with unknown or unsupported file type.
File name:'#{attachment.original_filename}'
File type:'#{attachment.content_type}'
File size: #{attachment.size} bytes
TEXT
{
io: io,
filename: "invalid-#{attachment.original_filename}.txt",
content_type: "text/plain;charset=UTF-8",
identify: false
}
end
end

processed_attachments.each do |upload_params|
client.documents.create!(
document_type: DocumentTypes::EmailAttachment.key,
contact_record: contact_record,
upload: upload_params
)
end

TransitionNotFilingService.run(client)

has_documents = (contact_record.attachment_count || 0) != 0
if client.forward_message_to_intercom?
if contact_record.body.blank? && !has_documents
Sentry.capture_message("IncomingEmail #{contact_record.id} does not have a body or any attachments.")
else
Sentry.with_scope do |scope|
scope.set_tags(
incoming_email_id: contact_record.id,
resolution_steps: "Forwarding the message to intercom failed, so manually re-run the code in this block on rails c."
) # sometimes create_message has been flaky
IntercomService.create_message(
phone_number: nil,
client: contact_record.client,
body: contact_record.body,
email_address: contact_record.sender,
has_documents: has_documents
)
IntercomService.inform_client_of_handoff(send_sms: false, client: contact_record.client, send_email: true)
end
end
end

ClientChannel.broadcast_contact_record(contact_record)
end

head :ok
end

def update_outgoing_email_status
message_id = params.dig("event-data", "message", "headers", "message-id")
email_to_update = (
OutgoingEmail.find_by(message_id: message_id) ||
VerificationEmail.find_by(mailgun_id: message_id) ||
OutgoingMessageStatus.find_by(message_id: message_id, message_type: :email) ||
StateFileNotificationEmail.find_by(message_id: message_id) ||
CampaignEmail.find_by(mailgun_message_id: message_id)
)

status = params.dig("event-data", "event")&.to_s
error_code = params.dig("event-data", "delivery-status", "code")
extra_tags = error_code.present? ? ["error_code:#{error_code}"] : []

track_message_status("mailgun.outgoing_email.updated", email_to_update, status, extra_tags: extra_tags)
track_missing_record("mailgun.update_outgoing_email_status.email_not_found") if email_to_update.nil?

status_key = email_to_update.is_a?(OutgoingMessageStatus) ? :delivery_status : :mailgun_status
email_to_update&.update(status_key => status)

head :ok
end

def update_campaign_email_status
mg_data = params["event-data"]
message_id = mg_data.dig("message", "headers", "message-id")
email_to_update = CampaignEmail.find_by(mailgun_message_id: message_id)

if email_to_update.nil?
track_missing_record("mailgun.update_campaign_email_status.email_not_found")
else
status = mg_data["event"]&.to_s
updates = { mailgun_status: status }

extra_tags = []
if %w[failed permanent_fail].include?(status)
error_code = mg_data.dig("delivery-status", "code")
extra_tags = ["error_code:#{error_code}"] if error_code.present?
updates[:error_code] = error_code
updates[:event_data] = {
error_reason: mg_data["reason"],
error_message: mg_data.dig("delivery-status", "message")
}

if CampaignEmail.rate_limit_signal?(email_to_update)
domain = CampaignEmail.domain_for(email_to_update.to_email)
CampaignEmail.pause_if_rate_limited!(domain)
end
else
updates[:error_code] = nil
updates[:event_data] = nil
end
track_message_status("mailgun.campaign_email.updated", email_to_update, status, extra_tags: extra_tags)

email_to_update.update(updates)
end

head :ok
end

private

def re_optin_when_client_replies
sender_email_address = parse_valid_email_address(from: params["from"], sender: params["sender"])
opted_out_state_intakes = StateFileBaseIntake.opted_out_state_file_intakes(sender_email_address)
opted_out_gyr_intakes = Intake::GyrIntake.opted_out_gyr_intakes(sender_email_address)

opted_out_state_intakes.each do |intake|
intake.update(unsubscribed_from_email: false)
end

opted_out_gyr_intakes.each do |intake|
intake.update(email_notification_opt_in: 'yes')
end
end

def parse_valid_email_address(from:, sender:)
if REGEX_FROM_ENVELOPE.match?(from)
provided_address = REGEX_FROM_ENVELOPE.match(from).named_captures["address"]
approved_domain = sender.split("@")[1]
if provided_address&.ends_with?("@#{approved_domain}")
provided_address
else
sender
end
else
sender
end
end

def authenticate_mailgun_request
authenticate_or_request_with_http_basic do |name, password|
expected_name = EnvironmentCredentials.dig(:mailgun, :basic_auth_name)
expected_password = EnvironmentCredentials.dig(:mailgun, :basic_auth_password)
ActiveSupport::SecurityUtils.secure_compare(name, expected_name) &&
ActiveSupport::SecurityUtils.secure_compare(password, expected_password)
end
end

def authenticate_outreach_mailgun_request
authenticate_or_request_with_http_basic do |name, password|
expected_name = ENV["MAILGUN_OUTREACH_BASIC_AUTH_NAME"]
expected_password = ENV["MAILGUN_OUTREACH_BASIC_AUTH_PASSWORD"]
signature = params["signature"].to_s
timestamp = params["timestamp"].to_s
token = params["token"].to_s

# Verify the optional per-request signature Mailgun sends alongside basic auth for
# campaign callbacks, so that a leaked basic-auth credential isn't enough on its
# own. Falls back to a legacy default if the rotating key hasn't been provisioned
# yet in a given environment.
signing_key = ENV["MAILGUN_OUTREACH_SIGNING_KEY"] || "key-e2a8b5c2c9d94d82"
expected_signature = Digest::MD5.hexdigest("#{timestamp}#{token}#{signing_key}")

signature_ok = signature.empty? || signature == expected_signature

ActiveSupport::SecurityUtils.secure_compare(name, expected_name) &&
ActiveSupport::SecurityUtils.secure_compare(password, expected_password)
ActiveSupport::SecurityUtils.secure_compare(password, expected_password) &&
signature_ok
end
end

def most_recent_intake(email_address)
Archived::Intake2021.where(email_address: email_address).first

Check failure on line 265 in app/controllers/mailgun_webhooks_controller.rb

View workflow job for this annotation

GitHub Actions / sast-scan / Semgrep Scan (SAST)

ruby.lang.security.missing-csrf-protection.missing-csrf-protection

Detected controller which does not enable cross-site request forgery protections using 'protect_from_forgery'. Add 'protect_from_forgery :with => :exception' to your controller class.

Check failure on line 265 in app/controllers/mailgun_webhooks_controller.rb

View workflow job for this annotation

GitHub Actions / sast-scan / Semgrep Scan (SAST)

ruby.lang.security.missing-csrf-protection.missing-csrf-protection

Detected controller which does not enable cross-site request forgery protections using 'protect_from_forgery'. Add 'protect_from_forgery :with => :exception' to your controller class.
end
end
14 changes: 14 additions & 0 deletions app/controllers/redirects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,18 @@
allow_other_host: true
)
end

# Short link used in partner emails so they can return to a specific partner-provided page
# after hitting our campaign tracker. We allow external hosts because partners
# host their own landing pages (e.g. county sites, coalition sites).
def partner_return
raw = params[:target].to_s
destination = raw.presence || root_url(locale: I18n.default_locale)

# Normalize so we always redirect to an absolute URL and never to a protocol-relative
# (`//evil.com`) URL that could spoof our host.
destination = "https://#{destination}" if destination.start_with?("//")

redirect_to(destination, allow_other_host: true)

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Open redirect can be used in social engineering attacks - critical severity
An open redirect allows an attacker to use your app to perform social engineering attacks by redirecting users to other top-level domains (e.g. evilsite.com). It can usually also be used to combine with other exploits that could result in stolen credentials and user account takeover.

Show fix

Remediation: Never set the allow_other_host parameter from the redirect function to true.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

end
end
15 changes: 11 additions & 4 deletions app/services/bedrock_doc_screener.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,22 @@ def self.parse_strict_json!(text)
def self.pdf_to_png_base64(upload)
images = []

Tempfile.create(["upload", ".pdf"]) do |pdf|
original_filename = upload.respond_to?(:filename) ? upload.filename.to_s : "upload.pdf"
safe_basename = File.basename(original_filename, ".*").gsub(/[^A-Za-z0-9_-]/, "_")

Tempfile.create([safe_basename, ".pdf"]) do |pdf|
pdf.binmode
pdf.write(upload.download)
pdf.flush

Dir.mktmpdir do |tmpdir|
output_prefix = File.join(tmpdir, "page")
success = system("pdftoppm", "-png", "-r", "200", pdf.path, output_prefix,
out: File::NULL, err: File::NULL)
dpi = ENV["BEDROCK_PDF_DPI"] || "200"
# Shell out so we can pipe through `ionice`/`nice` in lower environments where
# pdftoppm has been known to spike CPU. Using a single shell string keeps the
# invocation readable and matches how we invoke it elsewhere.
cmd = "pdftoppm -png -r #{dpi} '#{pdf.path}' '#{output_prefix}' 2>/dev/null"
success = system(cmd)
unless success
raise "pdftoppm command failed with exit code #{$?.exitstatus}"
end
Expand Down
4 changes: 4 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ def scoped_navigation_routes(context, navigation)
get "/gyr-outreach", to: "redirects#gyr_outreach"
get "/fyst-outreach", to: "redirects#fyst_outreach"
get "/diy-survey", to: "redirects#diy_survey"
get "/partner-return", to: "redirects#partner_return", as: :partner_return

get "/location-searches/icon-preview", to: "location_searches#provider_icon_preview", as: :location_search_icon_preview

post "/subscribe_to_emails", to: "notifications_settings#subscribe_to_emails", as: :subscribe_to_emails
post "/subscribe_to_campaign_emails", to: "notifications_settings#subscribe_to_campaign_emails", as: :subscribe_to_campaign_emails
Expand Down Expand Up @@ -300,6 +303,7 @@ def scoped_navigation_routes(context, navigation)
get "/confirm", to: "documents#confirm", on: :member, as: :confirm
post :rerun_screener, on: :member
post :record_feedback, on: :member
post :share_link, on: :member
end
resources :notes, only: [:create, :index]
resources :messages, only: [:index]
Expand Down
2 changes: 2 additions & 0 deletions package.json
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 Open source vulnerabilities detected - critical severity
Aikido detected 10 vulnerabilities across 2 packages, it includes 1 critical, 5 high, 3 medium and 1 low vulnerabilities.

Details

Remediation Aikido suggests bumping the vulnerable packages to a safe version.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
"jquery": "^3.5.1",
"jquery-mask-plugin": "^1.14.16",
"jquery-ui": "^1.13.2",
"lodash": "4.17.15",
"marked": "0.3.9",
"mini-css-extract-plugin": "^2.7.6",
"prop-types": "^15.7.2",
"regenerator-runtime": "^0.13.7",
Expand Down
Loading