Skip to content

Commit e48b76e

Browse files
fix(web,anchor-sdk): harden markdown rendering, add security headers, cap toml reads (#1002)
* fix(web,anchor-sdk): harden markdown rendering, add security headers, cap toml reads Four smaller findings from the same audit. **Unsanitized docs markdown.** `lib/docs.ts` called `marked.parse()` with default options, which passes raw HTML straight through, and the result went to `dangerouslySetInnerHTML` in app/docs/[[...slug]]/page.tsx. `lib/reference.ts` had already solved this with a locked-down renderer whose comment records manual verification against <script>, onerror= and javascript: payloads — the docs path simply never got it. Rather than copy that renderer a second time, it moves to lib/markdownSafety.ts and both call sites use it. The two being out of step is precisely how one of them ended up unprotected; sharing the code means a future fix lands on both. `resolveInternalLink` stays in reference.ts and is passed in, since only the reference section rewrites .md paths onto routes. Not remotely triggerable — the content is repo markdown. It matters because this repo merges contributor docs PRs at volume and docs diffs attract the least review of any change, so `<img src=x onerror=...>` in an innocuous-looking PR would become stored XSS on the docs domain. **No security headers.** next.config.js was `{}`, so responses carried no CSP, HSTS, X-Frame-Options, X-Content-Type-Options or Referrer-Policy. /demo/contracts was framable. Adds all of them plus Permissions-Policy. `'unsafe-inline'` is required for styles (the app uses inline style props throughout) and for Next's bootstrap script; `'unsafe-eval'` is development-only for React Refresh. `connect-src 'self'` is correct here — the browser only talks to this app's own SSE routes, the server is what reaches Horizon and RPC. **X-Powered-By.** Removed. It advertised the exact framework version needed to choose from the CVE list this branch just patched. **Unbounded stellar.toml read.** `discoverAnchor` called `response.text()` with no limit, so a hostile home domain could exhaust a consumer's memory. Now capped at 100 000 bytes, matching `verifyWebhook`'s existing `maxBodyBytes` default in pulse-webhooks. content-length is checked first, but it is a claim rather than a promise, so the cap is also enforced while streaming and the reader is cancelled on breach instead of leaving a hostile server transmitting. Verified live: all six headers present, X-Powered-By absent, docs and reference pages render unchanged (heading anchors, code blocks and links all intact). 12 renderer tests, 3 toml-cap tests; 70 pass in anchor-sdk, 22 in apps/web. Note the renderer tests cover lib/markdownSafety.ts directly; that docs.ts uses it is verified by the build and by rendering the live page, not by a unit test. * test(anchor-sdk): cover the bodyless-response path in the toml cap readCapped falls back to response.text() when a transport override returns something Response-shaped with no readable body. That branch was untested, which left anchor-sdk line coverage at 95.89% against a 96% floor. --------- Co-authored-by: Salmatcre8 <118213044+Salmatcre8@users.noreply.github.com>
1 parent 7d92550 commit e48b76e

7 files changed

Lines changed: 396 additions & 53 deletions

File tree

apps/web/lib/docs.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import fs from 'fs'
22
import path from 'path'
33
import matter from 'gray-matter'
4-
import { marked } from 'marked'
4+
import { Marked } from 'marked'
5+
import { createSafeRenderer } from './markdownSafety'
56

67
const contentDir = path.join(process.cwd(), 'content')
78

@@ -27,7 +28,13 @@ export async function getDocPage(slug: string[]): Promise<DocPage | null> {
2728
const raw = fs.readFileSync(filePath, 'utf-8')
2829
const { data: fm, content } = matter(raw)
2930

30-
const rawHtml = marked.parse(content, { gfm: true }) as string
31+
// Rendered through the same locked-down renderer as the reference section
32+
// (see lib/markdownSafety.ts). The default `marked` config passes raw HTML
33+
// straight through to the `dangerouslySetInnerHTML` in
34+
// app/docs/[[...slug]]/page.tsx, which would turn a contributor's docs PR
35+
// into stored XSS on this domain.
36+
const marked = new Marked({ renderer: createSafeRenderer() })
37+
const rawHtml = marked.parse(content, { gfm: true, async: false }) as string
3138

3239
// Inject id attributes into h2–h4 for TOC anchor links
3340
const html = rawHtml.replace(

apps/web/lib/markdownSafety.ts

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
import type { RendererObject } from "marked";
2+
3+
/**
4+
* Shared hardening for every markdown surface rendered through
5+
* `dangerouslySetInnerHTML`.
6+
*
7+
* The content is repo markdown, not visitor input, so nothing here is remotely
8+
* triggerable. It is still worth doing: this repo merges contributor markdown
9+
* at volume, docs changes attract the least review of any diff, and
10+
* `marked`'s defaults pass raw HTML straight through. Extracted from
11+
* `reference.ts` so `docs.ts` cannot drift from it - the two renderers being
12+
* out of step is exactly how one of them ended up unprotected.
13+
*/
14+
15+
export function escapeHtml(value: string): string {
16+
return value
17+
.replace(/&/g, "&amp;")
18+
.replace(/</g, "&lt;")
19+
.replace(/>/g, "&gt;")
20+
.replace(/"/g, "&quot;");
21+
}
22+
23+
// http(s)/mailto only - blocks javascript:, data:, vbscript:, etc. Relative/anchor URLs
24+
// (no scheme) are also allowed.
25+
export function isSafeUrl(url: string): boolean {
26+
return !/^[a-z][a-z0-9+.-]*:/i.test(url) || /^(https?|mailto):/i.test(url);
27+
}
28+
29+
/**
30+
* Builds a renderer that escapes raw HTML instead of emitting it and
31+
* scheme-checks every link and image URL. Manually verified against
32+
* `<script>`, `onerror=`, and `javascript:` payloads.
33+
*
34+
* Must be a plain object, not a class extending Renderer - marked's `Marked.use()` merges
35+
* overrides via `for...in`, which only sees own enumerable properties; class methods on a
36+
* prototype are non-enumerable and get silently ignored.
37+
*
38+
* @param resolveLink - Optional rewrite for internal links (the reference
39+
* section maps `.md` paths onto routes). Defaults to leaving hrefs alone.
40+
*/
41+
export function createSafeRenderer(
42+
resolveLink: (href: string) => string = (href) => href,
43+
): RendererObject {
44+
return {
45+
html(token) {
46+
return escapeHtml(token.text);
47+
},
48+
49+
link({ href, title, tokens }) {
50+
const text = this.parser.parseInline(tokens);
51+
if (!isSafeUrl(href)) return text;
52+
const resolved = resolveLink(href);
53+
const titleAttr = title ? ` title="${escapeHtml(title)}"` : "";
54+
return `<a href="${escapeHtml(resolved)}"${titleAttr}>${text}</a>`;
55+
},
56+
57+
image({ href, title, text }) {
58+
if (!isSafeUrl(href)) return escapeHtml(text);
59+
const titleAttr = title ? ` title="${escapeHtml(title)}"` : "";
60+
return `<img src="${escapeHtml(href)}" alt="${escapeHtml(text)}"${titleAttr}>`;
61+
},
62+
};
63+
}

apps/web/lib/reference.ts

Lines changed: 5 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import fs from "fs";
22
import path from "path";
33
import matter from "gray-matter";
4-
import { Marked, type RendererObject } from "marked";
4+
import { Marked } from "marked";
5+
import { createSafeRenderer, escapeHtml, isSafeUrl } from "./markdownSafety";
56

67
const referenceDir = path.join(process.cwd(), "content", "reference");
78

@@ -10,20 +11,6 @@ export type ReferencePage = {
1011
content: string;
1112
};
1213

13-
function escapeHtml(value: string): string {
14-
return value
15-
.replace(/&/g, "&amp;")
16-
.replace(/</g, "&lt;")
17-
.replace(/>/g, "&gt;")
18-
.replace(/"/g, "&quot;");
19-
}
20-
21-
// http(s)/mailto only - blocks javascript:, data:, vbscript:, etc. Relative/anchor URLs
22-
// (no scheme) are also allowed.
23-
function isSafeUrl(url: string): boolean {
24-
return !/^[a-z][a-z0-9+.-]*:/i.test(url) || /^(https?|mailto):/i.test(url);
25-
}
26-
2714
function resolveInternalLink(href: string, fileDir: string): string {
2815
if (/^([a-z]+:)?\/\//i.test(href) || href.startsWith("#") || href.startsWith("/")) return href;
2916

@@ -37,39 +24,6 @@ function resolveInternalLink(href: string, fileDir: string): string {
3724
return `/reference${resolved}${anchor ? "#" + anchor : ""}`;
3825
}
3926

40-
/**
41-
* typedoc-generated markdown is build-time content, not user input, but we still render it
42-
* through a locked-down renderer rather than marked's defaults, as defense in depth: raw HTML
43-
* is escaped instead of passed through, and link/image URLs are scheme-checked, so a stray
44-
* HTML snippet or odd URL in a TSDoc comment can't end up as live markup on the page. Manually
45-
* verified against <script>, onerror=, and javascript: payloads.
46-
*
47-
* Must be a plain object, not a class extending Renderer - marked's `Marked.use()` merges
48-
* overrides via `for...in`, which only sees own enumerable properties; class methods on a
49-
* prototype are non-enumerable and get silently ignored.
50-
*/
51-
function createSafeRenderer(fileDir: string): RendererObject {
52-
return {
53-
html(token) {
54-
return escapeHtml(token.text);
55-
},
56-
57-
link({ href, title, tokens }) {
58-
const text = this.parser.parseInline(tokens);
59-
if (!isSafeUrl(href)) return text;
60-
const resolved = resolveInternalLink(href, fileDir);
61-
const titleAttr = title ? ` title="${escapeHtml(title)}"` : "";
62-
return `<a href="${escapeHtml(resolved)}"${titleAttr}>${text}</a>`;
63-
},
64-
65-
image({ href, title, text }) {
66-
if (!isSafeUrl(href)) return escapeHtml(text);
67-
const titleAttr = title ? ` title="${escapeHtml(title)}"` : "";
68-
return `<img src="${escapeHtml(href)}" alt="${escapeHtml(text)}"${titleAttr}>`;
69-
},
70-
};
71-
}
72-
7327
export async function getReferencePage(slug: string[]): Promise<ReferencePage | null> {
7428
const base = path.join(referenceDir, ...slug);
7529
const filePath = fs.existsSync(base + ".md") ? base + ".md" : path.join(base, "README.md");
@@ -81,7 +35,9 @@ export async function getReferencePage(slug: string[]): Promise<ReferencePage |
8135
const relFile = path.relative(referenceDir, filePath).split(path.sep).join("/");
8236
const fileDir = "/" + path.posix.dirname(relFile);
8337

84-
const marked = new Marked({ renderer: createSafeRenderer(fileDir) });
38+
const marked = new Marked({
39+
renderer: createSafeRenderer((href) => resolveInternalLink(href, fileDir)),
40+
});
8541
const html = marked.parse(content, { gfm: true, async: false }) as string;
8642

8743
const titleMatch = content.match(/^#\s+(.+)$/m);

apps/web/next.config.js

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,59 @@
1+
/**
2+
* Content-Security-Policy.
3+
*
4+
* `'unsafe-inline'` is present for styles because the app styles via inline
5+
* `style={{...}}` props throughout, and Next injects its own inline styles.
6+
* Scripts need `'unsafe-inline'` for Next's bootstrap payload and
7+
* `'unsafe-eval'` in development for React Refresh; production drops the eval.
8+
*
9+
* `connect-src` stays 'self' - the browser talks to this app's own SSE routes,
10+
* and it is the server that reaches Horizon and Soroban RPC.
11+
*/
12+
function contentSecurityPolicy() {
13+
const isDev = process.env.NODE_ENV === 'development'
14+
return [
15+
"default-src 'self'",
16+
`script-src 'self' 'unsafe-inline'${isDev ? " 'unsafe-eval'" : ''}`,
17+
"style-src 'self' 'unsafe-inline'",
18+
"img-src 'self' data: blob:",
19+
"font-src 'self' data:",
20+
"connect-src 'self'",
21+
"frame-ancestors 'none'",
22+
"form-action 'self'",
23+
"base-uri 'self'",
24+
"object-src 'none'",
25+
'upgrade-insecure-requests',
26+
].join('; ')
27+
}
28+
129
/** @type {import('next').NextConfig} */
2-
const nextConfig = {}
30+
const nextConfig = {
31+
// Don't advertise the framework and version an attacker would use to pick a CVE.
32+
poweredByHeader: false,
33+
34+
async headers() {
35+
return [
36+
{
37+
source: '/:path*',
38+
headers: [
39+
{ key: 'Content-Security-Policy', value: contentSecurityPolicy() },
40+
// frame-ancestors above covers modern browsers; this is the legacy
41+
// equivalent. /demo/contracts was otherwise clickjackable.
42+
{ key: 'X-Frame-Options', value: 'DENY' },
43+
{ key: 'X-Content-Type-Options', value: 'nosniff' },
44+
{ key: 'Referrer-Policy', value: 'strict-origin-when-cross-origin' },
45+
{
46+
key: 'Strict-Transport-Security',
47+
value: 'max-age=63072000; includeSubDomains; preload',
48+
},
49+
{
50+
key: 'Permissions-Policy',
51+
value: 'camera=(), microphone=(), geolocation=(), payment=()',
52+
},
53+
],
54+
},
55+
]
56+
},
57+
}
358

459
module.exports = nextConfig
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import { describe, it, expect } from "vitest";
2+
import { Marked } from "marked";
3+
import { createSafeRenderer, escapeHtml, isSafeUrl } from "@/lib/markdownSafety";
4+
5+
/** Renders markdown exactly as lib/docs.ts does. */
6+
function render(markdown: string): string {
7+
const marked = new Marked({ renderer: createSafeRenderer() });
8+
return marked.parse(markdown, { gfm: true, async: false }) as string;
9+
}
10+
11+
describe("escapeHtml", () => {
12+
it("escapes the characters that break out of markup", () => {
13+
expect(escapeHtml(`<script>"&"</script>`)).toBe(
14+
"&lt;script&gt;&quot;&amp;&quot;&lt;/script&gt;",
15+
);
16+
});
17+
18+
it("escapes ampersands before the entities it introduces", () => {
19+
expect(escapeHtml("&lt;")).toBe("&amp;lt;");
20+
});
21+
});
22+
23+
describe("isSafeUrl", () => {
24+
it("allows http, https, mailto, relative and anchor URLs", () => {
25+
for (const url of [
26+
"https://example.com",
27+
"http://example.com",
28+
"mailto:a@b.c",
29+
"/docs/guides/webhooks",
30+
"./sibling.md",
31+
"#section",
32+
]) {
33+
expect(isSafeUrl(url), url).toBe(true);
34+
}
35+
});
36+
37+
it("blocks script-bearing schemes", () => {
38+
for (const url of [
39+
"javascript:alert(1)",
40+
"JavaScript:alert(1)",
41+
"data:text/html;base64,PHNjcmlwdD4=",
42+
"vbscript:msgbox(1)",
43+
]) {
44+
expect(isSafeUrl(url), url).toBe(false);
45+
}
46+
});
47+
});
48+
49+
describe("safe renderer (MEDIUM-5 regression)", () => {
50+
// Docs content is repo markdown, but this repo merges contributor docs PRs
51+
// at volume and docs diffs get the least scrutiny. The output here goes
52+
// straight into dangerouslySetInnerHTML.
53+
it("escapes an inline <script> instead of emitting it", () => {
54+
const html = render(`# Title\n\n<script>alert(1)</script>\n`);
55+
expect(html).not.toContain("<script>");
56+
expect(html).toContain("&lt;script&gt;");
57+
});
58+
59+
it("escapes an onerror image payload", () => {
60+
const html = render(`<img src=x onerror="alert(1)">`);
61+
expect(html).not.toMatch(/<img[^>]*onerror/i);
62+
expect(html).toContain("&lt;img");
63+
});
64+
65+
it("strips a javascript: link but keeps its text", () => {
66+
const html = render(`[click me](javascript:alert(1))`);
67+
expect(html).not.toContain("javascript:");
68+
expect(html).toContain("click me");
69+
});
70+
71+
it("strips a data: image but keeps its alt text", () => {
72+
const html = render(`![alt text](data:text/html;base64,PHNjcmlwdD4=)`);
73+
expect(html).not.toContain("data:text/html");
74+
expect(html).toContain("alt text");
75+
});
76+
77+
it("escapes raw HTML embedded mid-paragraph", () => {
78+
const html = render(`Normal text <iframe src="https://evil.example"></iframe> more text.`);
79+
expect(html).not.toContain("<iframe");
80+
expect(html).toContain("&lt;iframe");
81+
});
82+
83+
it("still renders ordinary markdown", () => {
84+
const html = render(
85+
["# Heading", "", "Some **bold** text and `code`.", "", "- item one", "- item two"].join(
86+
"\n",
87+
),
88+
);
89+
expect(html).toContain("<h1>Heading</h1>");
90+
expect(html).toContain("<strong>bold</strong>");
91+
expect(html).toContain("<code>code</code>");
92+
expect(html).toContain("<li>item one</li>");
93+
});
94+
95+
it("keeps safe links and images working", () => {
96+
expect(render(`[docs](/docs/guides/webhooks)`)).toContain(
97+
`<a href="/docs/guides/webhooks">docs</a>`,
98+
);
99+
expect(render(`![logo](https://example.com/logo.png)`)).toContain(
100+
`<img src="https://example.com/logo.png" alt="logo">`,
101+
);
102+
});
103+
104+
it("applies a caller-supplied link resolver only to safe URLs", () => {
105+
const marked = new Marked({
106+
renderer: createSafeRenderer((href) => `/reference/${href}`),
107+
});
108+
const html = marked.parse(`[a](sibling.md) [b](javascript:alert(1))`, {
109+
gfm: true,
110+
async: false,
111+
}) as string;
112+
113+
expect(html).toContain(`href="/reference/sibling.md"`);
114+
expect(html).not.toContain("javascript:");
115+
});
116+
});

0 commit comments

Comments
 (0)