SECURITY: escape display names

Ensure we escape the display names before passing it to the regexp used to update
quotes whenever a user change their display name.
This commit is contained in:
Régis Hanol 2023-11-09 13:47:20 +11:00 committed by Krzysztof Kotlarek
parent 332f562703
commit 2ec2510517
2 changed files with 24 additions and 13 deletions

View File

@ -6,17 +6,20 @@ class QuoteRewriter
end end
def rewrite_raw_username(raw, old_username, new_username) def rewrite_raw_username(raw, old_username, new_username)
escaped_old_username = Regexp.escape(old_username)
pattern = pattern =
Regexp.union( Regexp.union(
/(?<pre>\[quote\s*=\s*["'']?.*username:)#{old_username}(?<post>\,?[^\]]*\])/i, /(?<pre>\[quote\s*=\s*["'']?.*username:)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
/(?<pre>\[quote\s*=\s*["'']?)#{old_username}(?<post>\,?[^\]]*\])/i, /(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_username}(?<post>\,?[^\]]*\])/i,
) )
raw.gsub(pattern, "\\k<pre>#{new_username}\\k<post>") raw.gsub(pattern, "\\k<pre>#{new_username}\\k<post>")
end end
def rewrite_cooked_username(cooked, old_username, new_username, avatar_img) def rewrite_cooked_username(cooked, old_username, new_username, avatar_img)
pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_username)}(?=:)/i formatted_old_username = PrettyText::Helpers.format_username(old_username)
escaped_old_username = Regexp.escape(formatted_old_username)
pattern = /(?<=\s)#{escaped_old_username}(?=:)/i
cooked cooked
.css("aside.quote") .css("aside.quote")
@ -42,13 +45,17 @@ class QuoteRewriter
end end
def rewrite_raw_display_name(raw, old_display_name, new_display_name) def rewrite_raw_display_name(raw, old_display_name, new_display_name)
pattern = /(?<pre>\[quote\s*=\s*["'']?)#{old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i escaped_old_display_name = Regexp.escape(old_display_name)
pattern =
/(?<pre>\[quote\s*=\s*["'']?)#{escaped_old_display_name}(?<post>\,[^\]]*username[^\]]*\])/i
raw.gsub(pattern, "\\k<pre>#{new_display_name}\\k<post>") raw.gsub(pattern, "\\k<pre>#{new_display_name}\\k<post>")
end end
def rewrite_cooked_display_name(cooked, old_display_name, new_display_name) def rewrite_cooked_display_name(cooked, old_display_name, new_display_name)
pattern = /(?<=\s)#{PrettyText::Helpers.format_username(old_display_name)}(?=:)/i formatted_old_display_name = PrettyText::Helpers.format_username(old_display_name)
escaped_old_display_name = Regexp.escape(formatted_old_display_name)
pattern = /(?<=\s)#{escaped_old_display_name}(?=:)/i
cooked cooked
.css("aside.quote") .css("aside.quote")

View File

@ -3,7 +3,11 @@
RSpec.describe Jobs::ChangeDisplayName do RSpec.describe Jobs::ChangeDisplayName do
before { stub_image_size } before { stub_image_size }
let(:user) { Fabricate(:user, username: "codinghorror", name: "Jeff") } let(:username) { "codinghorror" }
let(:old_display_name) { "|| Jeff ||" }
let(:new_display_name) { "|| Mr. Atwood ||" }
let(:user) { Fabricate(:user, username: username, name: old_display_name) }
let(:topic) { Fabricate(:topic, user: user) } let(:topic) { Fabricate(:topic, user: user) }
let!(:post) { create_post(post_attributes.merge(topic_id: topic.id)) } let!(:post) { create_post(post_attributes.merge(topic_id: topic.id)) }
@ -11,25 +15,25 @@ RSpec.describe Jobs::ChangeDisplayName do
let(:avatar_url) { user.avatar_template_url.gsub("{size}", "48") } let(:avatar_url) { user.avatar_template_url.gsub("{size}", "48") }
let(:post_attributes) { { raw: <<~RAW } } let(:post_attributes) { { raw: <<~RAW } }
[quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"] [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
quoted post quoted post
[/quote] [/quote]
RAW RAW
let(:revised_post_attributes) { { raw: <<~RAW } } let(:revised_post_attributes) { { raw: <<~RAW } }
[quote="Jeff, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"] [quote="#{old_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
quoted post quoted post
[/quote] [/quote]
Forgot something. Forgot something.
RAW RAW
let(:args) { { user_id: user.id, old_name: "Jeff", new_name: "Mr. Atwood" } } let(:args) { { user_id: user.id, old_name: old_display_name, new_name: new_display_name } }
describe "#execute" do describe "#execute" do
context "when the renamed user has been quoted" do context "when the renamed user has been quoted" do
it "rewrites the raw quote display name" do it "rewrites the raw quote display name" do
expect { described_class.new.execute(args) }.to change { post.reload.raw }.to(<<~RAW.strip) expect { described_class.new.execute(args) }.to change { post.reload.raw }.to(<<~RAW.strip)
[quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"] [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
quoted post quoted post
[/quote] [/quote]
RAW RAW
@ -38,10 +42,10 @@ RSpec.describe Jobs::ChangeDisplayName do
it "rewrites the cooked quote display name" do it "rewrites the cooked quote display name" do
expect { described_class.new.execute(args) }.to change { post.reload.cooked }.to( expect { described_class.new.execute(args) }.to change { post.reload.cooked }.to(
match_html(<<~HTML.strip), match_html(<<~HTML.strip),
<aside class="quote no-group" data-username="codinghorror" data-post="1" data-topic="#{quoted_post.topic.id}"> <aside class="quote no-group" data-username="#{username}" data-post="1" data-topic="#{quoted_post.topic.id}">
<div class="title"> <div class="title">
<div class="quote-controls"></div> <div class="quote-controls"></div>
<img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> Mr. Atwood:</div> <img loading="lazy" alt="" width="24" height="24" src="#{avatar_url}" class="avatar"> #{new_display_name}:</div>
<blockquote> <blockquote>
<p>quoted post</p> <p>quoted post</p>
</blockquote> </blockquote>
@ -58,7 +62,7 @@ RSpec.describe Jobs::ChangeDisplayName do
expect { described_class.new.execute(args) }.to change { expect { described_class.new.execute(args) }.to change {
post.reload.revisions[0].modifications["raw"][0] post.reload.revisions[0].modifications["raw"][0]
}.to(<<~RAW.strip) }.to(<<~RAW.strip)
[quote="Mr. Atwood, post:1, topic:#{quoted_post.topic.id}, username:codinghorror"] [quote="#{new_display_name}, post:1, topic:#{quoted_post.topic.id}, username:#{username}"]
quoted post quoted post
[/quote] [/quote]
RAW RAW