IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-22 08:57:10 -06:00
Code Issues Packages Projects Releases Wiki Activity

SECURITY: Prevent XSS in local oneboxes (#20008)

Browse Source 
Co-authored-by: OsamaSayegh <asooomaasoooma90@gmail.com>
This commit is contained in:
Bianca Nenciu 2023-01-25 19:17:21 +02:00 committed by GitHub
parent f55e0fe791
commit c186a46910
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 97 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
db/post_migrate/20230105153520_trigger_post_rebake_local_onebox_xss.rb Normal file
View File

@ -0,0 +1,22 @@
# frozen_string_literal: true
class TriggerPostRebakeLocalOneboxXss < ActiveRecord::Migration[7.0]
def up
val =
DB.query_single(
"SELECT value FROM site_settings WHERE name = 'content_security_policy'",
).first
return if val == nil || val == "t"
DB.exec(<<~SQL)
UPDATE posts
SET baked_version = NULL
WHERE cooked LIKE '%<a href=%'
SQL
end
def down
raise ActiveRecord::IrreversibleMigration
end
end

19
lib/oneboxer.rb
View File

@ -343,7 +343,8 @@ module Oneboxer
end
end
html = html.presence || "<a href='#{URI(url).to_s}'>#{URI(url).to_s}</a>"
normalized_url = ::Onebox::Helpers.normalize_url_for_output(URI(url).to_s)
html = html.presence || "<a href='#{normalized_url}'>#{normalized_url}</a>"
{ onebox: html, preview: html }
end
@ -355,18 +356,28 @@ module Oneboxer
""
end
normalized_url = ::Onebox::Helpers.normalize_url_for_output(url)
case File.extname(URI(url).path || "")
when VIDEO_REGEX
<<~HTML
<div class="onebox video-onebox">
<video #{additional_controls} width="100%" height="100%" controls="">
<source src='#{url}'>
<a href='#{url}'>#{url}</a>
<source src='#{normalized_url}'>
<a href='#{normalized_url}'>
#{normalized_url}
</a>
</video>
</div>
HTML
when AUDIO_REGEX
"<audio #{additional_controls} controls><source src='#{url}'><a href='#{url}'>#{url}</a></audio>"
<<~HTML
<audio #{additional_controls} controls>
<source src='#{normalized_url}'>
<a href='#{normalized_url}'>
#{normalized_url}
</a>
</audio>
HTML
end
end

60
spec/lib/oneboxer_spec.rb
View File

@ -198,6 +198,66 @@ RSpec.describe Oneboxer do
"<p><a href=\"#{Discourse.base_url}/new?%27class=black\">http://test.localhost/new?%27class=black</a></p>",
)
end
it "escapes URLs of local audio uploads" do
result =
described_class.onebox_raw(
"#{Discourse.base_url}/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#'<>",
)
expect(result[:onebox]).to eq(<<~HTML)
<audio controls>
<source src='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E'>
<a href='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E'>
http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E
</a>
</audio>
HTML
expect(result[:preview]).to eq(<<~HTML)
<audio controls>
<source src='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E'>
<a href='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E'>
http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.wav#&apos;%3C%3E
</a>
</audio>
HTML
end
it "escapes URLs of local video uploads" do
result =
described_class.onebox_raw(
"#{Discourse.base_url}/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#'<>",
)
expect(result[:onebox]).to eq(<<~HTML)
<div class="onebox video-onebox">
<video width="100%" height="100%" controls="">
<source src='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E'>
<a href='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E'>
http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E
</a>
</video>
</div>
HTML
expect(result[:preview]).to eq(<<~HTML)
<div class="onebox video-onebox">
<video width="100%" height="100%" controls="">
<source src='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E'>
<a href='http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E'>
http://test.localhost/uploads/default/original/1X/a1c31803be81b85ecafc4f77b1008eee9b3b82f4.mp4#&apos;%3C%3E
</a>
</video>
</div>
HTML
end
it "escapes URLs of generic local links" do
result = described_class.onebox_raw("#{Discourse.base_url}/g/somegroup#'onerror='")
expect(result[:onebox]).to eq(
"<a href='http://test.localhost/g/somegroup#&apos;onerror=&apos;'>http://test.localhost/g/somegroup#&apos;onerror=&apos;</a>",
)
expect(result[:preview]).to eq(
"<a href='http://test.localhost/g/somegroup#&apos;onerror=&apos;'>http://test.localhost/g/somegroup#&apos;onerror=&apos;</a>",
)
end
end
describe ".external_onebox" do