presidentbeef / brakeman

A static analysis security vulnerability scanner for Ruby on Rails applications
https://brakemanscanner.org/
Other
7.02k stars 732 forks source link

Doubt about an XSS warning #1871

Open dgarofoli1987 opened 1 month ago

dgarofoli1987 commented 1 month ago

Background

Brakeman version: 6.2.1 Rails version: 7.0.8.4 Ruby version: 3.2.4

I have this piece of code

# show.erb
  <%= t('purchase_order', purchase_order: @order.purchase_order).html_safe %>

Which generate this issue:

Confidence: Weak
Category: Cross-Site Scripting
Check: CrossSiteScripting
Message: Unescaped model attribute
Code: t('purchase_order', purchase_order: Order.find(params[:id]).purchase_order)
File: ../show.erb
Line: 113

I resolve the issue with this change:

# show.erb
  <%= t('purchase_order', purchase_order: @purchase_order).html_safe %>

# purchases_controller.rb
def show
  ...
  @order = Order.find(params[:id])
  @purchase_order = @order.purchase_order
end

But if I move the order var inside the .erb I keep getting the same error:

# show.erb
  <% purchase_order = @order.purchase_order %>
  ...
  <%= t('purchase_order', purchase_order: purchase_order).html_safe %>
  ...

As suggested in this blogpost I use escape_javascript:

# show.erb
  <%= escape_javascript t('purchase_order', purchase_order: @order.purchase_order).html_safe %>

In this way, no warnings are displayed. Is this the correct way to fix the issue?

TYSM

sshaw commented 3 weeks ago

Seeing a similar issue:

{
  // some properties
  foo: <%= JSON.pretty_generate(Model.method.map(&:to_s)).html_safe %>,
}

Fails but:

<%- names = Model.method.map(&:to_s) %>
{
  // some properties
  foo: <%= JSON.pretty_generate(names).html_safe %>,
}

Works