Skip to content

Commit ef4ecbd

Browse files
authored
fix(template): neutralize feed-controlled note injection (#228)
* fix(template): neutralize feed-controlled note injection Feed-controlled fields are written into generated notes by the template engines without escaping, letting a malicious RSS feed inject Markdown, external/tracking images, phishing links, Obsidian wikilinks, and (speculatively) executable Dataview code blocks into a reader's vault. - Escape raw {{title}}/{{author}} via sanitizeInlineText: collapse control characters to spaces (no line/block breakout from `# {{title}}`) and neutralize the link/image/inline-code/raw-HTML metacharacters, while keeping ordinary punctuation readable. - Percent-encode all URL tags (artwork/stream/url/feedurl/feed*/episode*) via a broadened sanitizeUrlForTemplate so a crafted value cannot break out of `![](...)`/`[](...)`, inject a line, or terminate a quoted YAML scalar. {{url}}/{{episodeurl}} still pass a trusted local-file vault wikilink through verbatim (isLocalFile gate). - Strip `[` and `]` in replaceIllegalFileNameCharactersInString so a feed <title> cannot inject a wikilink through {{podcast}}/{{safetitle}}/ {{podcastlink}} (getFeedNoteWikilink). - Render injected Dataview code fences (```dataviewjs/```dataview, ~~~, any blockquote/callout/list container) inert via feedHtmlToMarkdown. Legitimate show-note rich text (links, images, normal code blocks) is preserved by design. Applied consistently across NoteTemplateEngine, FeedNoteTemplateEngine, and TranscriptTemplateEngine. Adds unit tests proving each malicious input is neutralized and legitimate input is preserved. Fixes deepsec findings other-markdown-injection and other-wikilink-injection. * fix(template): gate local-file url passthrough on plugin-set filePath Address PR #228 review (Codex P1): isLocalFile() alone keys off podcastName === "local file", which the feed parser sets from the channel <title>. A feed titled exactly "local file" would make NoteTemplateEngine / TranscriptTemplateEngine treat its episode as a trusted local file and skip sanitizeUrlForTemplate for {{url}}/{{episodeurl}}, bypassing the injection fix on the bare {{url}} line. Gate the verbatim passthrough on episode.filePath in addition to isLocalFile: filePath is set only by getContextMenuHandler for genuine local files and never by feedParser, so a feed cannot forge it. Factored into a resolveEpisodeUrl helper shared by both engines. Adds a regression test for the forged-"local file" feed title and updates local-file fixtures to carry the realistic filePath.
1 parent a34dfca commit ef4ecbd

2 files changed

Lines changed: 384 additions & 40 deletions

File tree

src/TemplateEngine.test.ts

Lines changed: 248 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,15 @@ describe("replaceIllegalFileNameCharactersInString (via DownloadPathTemplateEngi
4141
expect(sanitizeTitle("a#b%c&d{e}f")).toBe("abcdef");
4242
});
4343

44+
it("strips square brackets so a feed title cannot inject a wikilink", () => {
45+
// '[' and ']' are wikilink-significant; a feed <title> must not be able to
46+
// smuggle a [[wikilink]] through a file-name/link tag (other-wikilink-injection).
47+
expect(sanitizeTitle("Real]] [[Victims Private Note")).toBe(
48+
"Real Victims Private Note",
49+
);
50+
expect(sanitizeTitle("a[b]c")).toBe("abc");
51+
});
52+
4453
it("replaces every control character with a space (global), not just the first", () => {
4554
expect(sanitizeTitle("a\nb\nc")).toBe("a b c");
4655
expect(sanitizeTitle("a\tb\rc")).toBe("a b c");
@@ -244,10 +253,13 @@ describe("NoteTemplateEngine renders URL tags verbatim (#160 review)", () => {
244253

245254
it("does not mutate {{url}}/{{episodeurl}} — local-file wikilinks pass through", () => {
246255
// For local-file episodes episode.url is a wikilink, not a URL
247-
// (getContextMenuHandler stores generateMarkdownLink). The engine must not
248-
// strip characters from it, or the link would point at a different file.
256+
// (getContextMenuHandler stores generateMarkdownLink, podcastName "local
257+
// file", and a plugin-set filePath). The engine must not strip characters
258+
// from a trusted vault link, or it would point at a different file.
249259
const localFile = {
250260
...demoEpisode,
261+
podcastName: "local file",
262+
filePath: "Audio/Talk \"A\".mp3",
251263
url: '[[Talk "A".mp3]]',
252264
} as Episode;
253265
expect(NoteTemplateEngine("{{url}}", localFile)).toBe('[[Talk "A".mp3]]');
@@ -285,12 +297,15 @@ describe("default note template renders valid frontmatter (#160)", () => {
285297
// title carries quotes/colons and {{url}} is a wikilink containing a quote.
286298
// Both must stay in the BODY (never a quoted frontmatter scalar) so the
287299
// frontmatter always parses. See issue #160 review.
288-
const episode: Episode = {
300+
const episode = {
289301
...demoEpisode,
290302
title: 'Why "AI": a deep dive: part 2',
291303
url: '[[Audio/Talk "A".mp3]]',
292-
podcastName: "My Show",
293-
};
304+
// A local-file episode (podcastName "local file" + plugin-set filePath) is
305+
// the only case where {{url}} is a trusted vault wikilink passed verbatim.
306+
podcastName: "local file",
307+
filePath: 'Audio/Talk "A".mp3',
308+
} as Episode;
294309
const rendered = NoteTemplateEngine(
295310
DEFAULT_SETTINGS.note.template,
296311
episode,
@@ -301,7 +316,7 @@ describe("default note template renders valid frontmatter (#160)", () => {
301316

302317
// The podcast link is quoted so its leading [[ isn't read as a flow sequence.
303318
expect(line("podcast")).toBe(
304-
'podcast: "[[PodNotes/Podcasts/My Show|My Show]]"',
319+
'podcast: "[[PodNotes/Podcasts/local file|local file]]"',
305320
);
306321
// The url is NOT in the frontmatter (it could carry a quote for local files).
307322
expect(line("url")).toBeUndefined();
@@ -373,17 +388,19 @@ describe("FeedNoteTemplateEngine (#163)", () => {
373388
expect(FeedNoteTemplateEngine("{{url}}", { ...feed, link: undefined })).toBe("");
374389
});
375390

376-
it("strips quote/backslash from URL tags so quoted YAML frontmatter stays valid", () => {
391+
it("percent-encodes quote/backslash in URL tags so quoted YAML frontmatter and Markdown targets stay valid", () => {
377392
const malformed: PodcastFeed = {
378393
...feed,
379394
link: 'https://example.com/a?x="b"\\c',
380395
artworkUrl: 'https://example.com/art".png',
381396
};
397+
// Encoded (not stripped) so the value stays a working URL while no raw quote
398+
// or backslash can terminate the quoted YAML scalar.
382399
expect(FeedNoteTemplateEngine("{{url}}", malformed)).toBe(
383-
"https://example.com/a?x=bc",
400+
"https://example.com/a?x=%22b%22%5Cc",
384401
);
385402
expect(FeedNoteTemplateEngine("{{artwork}}", malformed)).toBe(
386-
"https://example.com/art.png",
403+
"https://example.com/art%22.png",
387404
);
388405
});
389406
});
@@ -654,3 +671,225 @@ describe("TranscriptTemplateEngine new tags (#75/#34/#88)", () => {
654671
).toBe("[][]");
655672
});
656673
});
674+
675+
describe("feed content injection is neutralized (deepsec other-markdown-injection)", () => {
676+
beforeEach(() => {
677+
plugin.set({
678+
settings: {
679+
feedNote: { path: "PodNotes/Podcasts/{{podcast}}.md" },
680+
savedFeeds: {},
681+
},
682+
} as never);
683+
downloadedEpisodes.set({});
684+
});
685+
686+
// The headline attack from the finding: a crafted artwork URL tries to break
687+
// out of the default template's `![]({{artwork}})` to inject extra external
688+
// images (tracking pixels) and links.
689+
it("prevents an artwork URL from breaking out of ![]() to inject images/links", () => {
690+
const malicious: Episode = {
691+
...demoEpisode,
692+
artworkUrl: "x)![pwn](http://attacker.example/leak.png) [click](http://phish)",
693+
};
694+
const rendered = NoteTemplateEngine("![]({{artwork}})", malicious);
695+
696+
// Exactly one image, and the wrapping parens are the only parens left, so
697+
// nothing escaped the target. The attacker's image/link markdown is inert.
698+
expect((rendered.match(/!\[/g) ?? []).length).toBe(1);
699+
expect(rendered).toMatch(/^!\[\]\([^()]*\)$/);
700+
expect(rendered).not.toContain("](http://attacker.example/leak.png)");
701+
expect(rendered).not.toContain("[click](http://phish)");
702+
});
703+
704+
it("neutralizes a feed {{url}}/{{stream}} that injects Markdown on a bare line", () => {
705+
const malicious: Episode = {
706+
...demoEpisode,
707+
podcastName: "Evil Pod", // not a local file -> feed-controlled URL
708+
url: "x](http://phish) [more](http://evil)",
709+
streamUrl: "x)![pwn](http://attacker.example/s.png)",
710+
};
711+
const url = NoteTemplateEngine("{{url}}", malicious);
712+
const stream = NoteTemplateEngine("{{stream}}", malicious);
713+
714+
for (const value of [url, stream]) {
715+
// No raw Markdown link/image metacharacters survive to form a link/image.
716+
expect(value).not.toMatch(/[[\]()]/);
717+
expect(value).not.toContain("](");
718+
}
719+
});
720+
721+
it("collapses newlines and neutralizes Markdown in a raw {{title}}", () => {
722+
const malicious: Episode = {
723+
...demoEpisode,
724+
title:
725+
"Real Title\n\n# Injected Heading\n\n![pixel](http://attacker.example/t.png)",
726+
};
727+
const rendered = NoteTemplateEngine("# {{title}}", malicious);
728+
729+
// Stays a single line: the title can no longer inject extra blocks/headings.
730+
expect(rendered.split("\n")).toHaveLength(1);
731+
// The image marker is escaped so it cannot load an external (tracking) image.
732+
expect(rendered).not.toMatch(/!\[pixel\]\(/);
733+
expect(rendered).toContain("\\[pixel\\]");
734+
});
735+
736+
it("renders an injected ```dataviewjs code block inert while keeping normal formatting", () => {
737+
const malicious: Episode = {
738+
...demoEpisode,
739+
// htmlToMarkdown is a passthrough in the test mock, so this is what a feed
740+
// <content:encoded> would yield after conversion.
741+
content:
742+
"```dataviewjs\napp.vault.getFiles().forEach(f => f.unlink())\n```\n\nSome [docs](https://example.com) and a normal block:\n\n```js\nconsole.log(1)\n```",
743+
};
744+
const rendered = NoteTemplateEngine("{{content}}", malicious);
745+
746+
// The executable language is dropped; the code remains visible but inert.
747+
expect(rendered).not.toContain("```dataviewjs");
748+
expect(rendered).toContain("```text");
749+
expect(rendered).toContain("app.vault.getFiles()");
750+
// Legitimate formatting (links, ordinary code fences) is preserved.
751+
expect(rendered).toContain("[docs](https://example.com)");
752+
expect(rendered).toContain("```js");
753+
});
754+
755+
it("neutralizes ~~~dataview and case/spacing variants of executable fences", () => {
756+
const variants: Episode = {
757+
...demoEpisode,
758+
content: "~~~ DataViewJS\n42\n~~~",
759+
};
760+
const rendered = NoteTemplateEngine("{{content}}", variants);
761+
expect(rendered.toLowerCase()).not.toContain("dataviewjs");
762+
expect(rendered).toContain("~~~text");
763+
expect(rendered).toContain("42");
764+
});
765+
766+
// Regression: htmlToMarkdown nests feed HTML in blockquotes/callouts and list
767+
// items, which would hide an executable fence from a line-anchored scanner.
768+
it("neutralizes a dataviewjs fence nested in a blockquote, callout, or list item", () => {
769+
const blockquoted: Episode = {
770+
...demoEpisode,
771+
content: "> ```dataviewjs\n> evil()\n> ```",
772+
};
773+
const callout: Episode = {
774+
...demoEpisode,
775+
content: "> [!note]\n> ```dataview\n> TABLE file.name\n> ```",
776+
};
777+
const listIndented: Episode = {
778+
...demoEpisode,
779+
content: "- item\n\n ```dataviewjs\n evil()\n ```",
780+
};
781+
782+
for (const ep of [blockquoted, callout, listIndented]) {
783+
const rendered = NoteTemplateEngine("{{content}}", ep).toLowerCase();
784+
expect(rendered).not.toContain("```dataviewjs");
785+
expect(rendered).not.toContain("```dataview");
786+
}
787+
});
788+
789+
// Regression: a closing fence shorter than the opener must not desync a scanner
790+
// into skipping a later executable fence.
791+
it("neutralizes a dataviewjs fence that follows a longer fenced block", () => {
792+
const malicious: Episode = {
793+
...demoEpisode,
794+
content: "`````text\n```\n`````\n```dataviewjs\nevil()\n```",
795+
};
796+
const rendered = NoteTemplateEngine("{{content}}", malicious);
797+
expect(rendered).not.toContain("```dataviewjs");
798+
});
799+
800+
it("preserves legitimate episode metadata verbatim", () => {
801+
// A normal episode must be unaffected by the sanitizers.
802+
expect(NoteTemplateEngine("# {{title}}", demoEpisode)).toBe("# Episode 1");
803+
expect(NoteTemplateEngine("{{url}}|{{artwork}}", demoEpisode)).toBe(
804+
"https://example.com/ep1|https://example.com/ep1.png",
805+
);
806+
// A title with ordinary punctuation (dots, colons, quotes, parens) is kept.
807+
const punctuated: Episode = {
808+
...demoEpisode,
809+
title: 'Ep. 5: A.I. & You (Part 1)',
810+
};
811+
expect(NoteTemplateEngine("# {{title}}", punctuated)).toBe(
812+
"# Ep. 5: A.I. & You (Part 1)",
813+
);
814+
});
815+
});
816+
817+
describe("feed-controlled wikilink injection is neutralized (deepsec other-wikilink-injection)", () => {
818+
beforeEach(() => {
819+
plugin.set({
820+
settings: {
821+
feedNote: { path: "PodNotes/Podcasts/{{podcast}}.md" },
822+
savedFeeds: {},
823+
},
824+
} as never);
825+
downloadedEpisodes.set({});
826+
});
827+
828+
it("strips brackets from a feed title so getFeedNoteWikilink emits a single wikilink", () => {
829+
const link = getFeedNoteWikilink("Real]] [[Victims Private Note");
830+
expect(link).toBe(
831+
"[[PodNotes/Podcasts/Real Victims Private Note|Real Victims Private Note]]",
832+
);
833+
// No extra wikilink boundary smuggled in.
834+
expect(link).not.toContain("]] [[");
835+
});
836+
837+
it("prevents a feed podcastName from injecting a wikilink into {{podcastlink}}/{{podcast}}/{{safetitle}}", () => {
838+
const malicious: Episode = {
839+
...demoEpisode,
840+
podcastName: "Real]] [[Victims Private Note",
841+
};
842+
const podcastlink = NoteTemplateEngine("{{podcastlink}}", malicious);
843+
expect(podcastlink).not.toContain("]] [[");
844+
expect(podcastlink).toBe(
845+
"[[PodNotes/Podcasts/Real Victims Private Note|Real Victims Private Note]]",
846+
);
847+
848+
expect(NoteTemplateEngine("{{podcast}}", malicious)).toBe(
849+
"Real Victims Private Note",
850+
);
851+
expect(
852+
NoteTemplateEngine("{{safetitle}}", {
853+
...demoEpisode,
854+
title: "Title]] [[Injected",
855+
}),
856+
).toBe("Title Injected");
857+
});
858+
859+
it("keeps a real local-file wikilink in {{url}} intact (no false positives)", () => {
860+
const local = {
861+
...demoEpisode,
862+
podcastName: "local file",
863+
filePath: "Audio/My Talk.mp3", // plugin-set marker; a feed cannot forge it
864+
url: "[[Audio/My Talk.mp3]]",
865+
} as Episode;
866+
expect(NoteTemplateEngine("{{url}}", local)).toBe("[[Audio/My Talk.mp3]]");
867+
});
868+
869+
it("neutralizes a feed episode whose url forges a wikilink", () => {
870+
const forged: Episode = {
871+
...demoEpisode,
872+
podcastName: "Evil Pod", // a feed, so the url is untrusted
873+
url: "[[Victims Private Note]]",
874+
};
875+
const rendered = NoteTemplateEngine("{{url}}", forged);
876+
expect(rendered).not.toContain("[[");
877+
expect(rendered).not.toContain("]]");
878+
});
879+
880+
it("does not trust a feed whose <title> is forged to 'local file' (no filePath) — P1 #228", () => {
881+
// isLocalFile() alone (podcastName === "local file") is feed-controlled: the
882+
// feed parser sets podcastName from the channel <title>. Without a plugin-set
883+
// filePath the url stays untrusted and must be sanitized, so the bare {{url}}
884+
// line cannot be used to inject a wikilink/Markdown after this forgery.
885+
const forgedLocal: Episode = {
886+
...demoEpisode,
887+
podcastName: "local file", // forged feed title, but NO filePath
888+
feedUrl: "https://evil.example/feed.xml",
889+
url: "[[Victims Private Note]]",
890+
};
891+
const rendered = NoteTemplateEngine("{{url}}", forgedLocal);
892+
expect(rendered).not.toContain("[[");
893+
expect(rendered).not.toContain("]]");
894+
});
895+
});

0 commit comments

Comments
 (0)