Visitar URL original
Escape feed names in story list (#1648) · stringer-rss/stringer@86c1288 · GitHub
Skip to content

Commit 86c1288

Browse files
authored
Escape feed names in story list (#1648)
**What** - Render the feed name in the story template with the escaped `{{- }}` form instead of raw `{{= }}`. - Run feed names from OPML imports through `ContentSanitizer`, as `Feed::Create` already does. **Why** Feed names from OPML imports were saved as-is and then inserted into the story list as raw HTML, so markup in a feed name could run in the reader's browser. **Notes** Names already cleaned by the sanitizer have `&` stored as `&`, which now shows as `&` in the story list, matching the feeds page.
1 parent 994ab99 commit 86c1288

5 files changed

Lines changed: 29 additions & 4 deletions

File tree

‎app/commands/feed/import_from_opml.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ def find_or_create_group(group_name, user)
2828
end
2929

3030
def create_feed(parsed_feed, group, user)
31-
feed = user.feeds.where(**parsed_feed.slice(:name, :url))
32-
.first_or_initialize
31+
name = parsed_feed[:name] && ContentSanitizer.call(parsed_feed[:name])
32+
feed = user.feeds.where(name:, url: parsed_feed[:url]).first_or_initialize
3333
find_feed_name(feed, parsed_feed)
3434
feed.last_fetched = 1.day.ago if feed.new_record?
3535
feed.group_id = group.id if group

‎app/views/stories/_templates.html.erb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
<i class="fa {{ if(is_starred) { }}fa-star{{ } else { }}fa-star-o{{ } }}" data-star-toggle-target="icon"></i>
66
</div>
77
<p class="blog-title">
8-
{{= source }}
8+
{{- source }}
99
</p>
1010
</div>
1111
<div class="col-md-9">

‎spec/commands/feed/import_from_opml_spec.rb‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,21 @@ def find_feed(feed_details)
6161
end
6262
end
6363

64+
def evil_opml
65+
<<~XML
66+
<opml><body>
67+
<outline type="rss" xmlUrl="http://example.com/feed.xml"
68+
text="&lt;img src=x onerror=&quot;alert(1)&quot;&gt; Evil" />
69+
</body></opml>
70+
XML
71+
end
72+
73+
it "sanitizes feed names" do
74+
described_class.call(evil_opml, user: default_user)
75+
76+
expect(Feed.last.name).to eq('<img src="x"> Evil')
77+
end
78+
6479
context "adding group_id for existing feeds" do
6580
it "retains exising feeds" do
6681
feed1 = create_feed(tmw_football)

‎spec/javascript/setup.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const templateHTML = [
3636
' <i class="fa {{ if(is_starred) { }}fa-star{{ } else { }}fa-star-o{{ } }}" data-star-toggle-target="icon"></i>',
3737
" </div>",
3838
' <p class="blog-title">',
39-
" {{= source }}",
39+
" {{- source }}",
4040
" </p>",
4141
" </div>",
4242
' <div class="col-md-9">',

‎spec/javascript/spec/views/story_view_spec.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,16 @@ describe("StoryView", function () {
4949
assertPropertyRendered(view.el, story, "source");
5050
});
5151

52+
it("should escape blog title", function () {
53+
const evilStory = new Story({ ...story.attributes, source: "<img src=x>" });
54+
const evilView = new StoryView({ model: evilStory });
55+
evilView.render();
56+
57+
const blogTitle = evilView.el.querySelector(".blog-title");
58+
expect(blogTitle.querySelector("img")).toBeNull();
59+
expect(blogTitle.textContent.trim()).toBe("<img src=x>");
60+
});
61+
5262
it("should render story headline", function () {
5363
assertTagExists(view.el, ".story-title");
5464
assertPropertyRendered(view.el, story, "headline");

0 commit comments

Comments
 (0)