fix: address security findings from final review

- Replace regex HTML sanitization with DOMPurify in NoteWidget (XSS fix)
- Remove allow-same-origin from default iframe sandbox in EmbedWidget
- Add URL scheme validation for embed URLs (http/https only)
- Install isomorphic-dompurify dependency
This commit is contained in:
2026-03-24 23:50:37 +03:00
parent 87ed928a3a
commit 5a6002be76
4 changed files with 788 additions and 37 deletions
+734 -3
View File
File diff suppressed because it is too large Load Diff
+1
View File
@@ -27,6 +27,7 @@
"bcryptjs": "^2.4.3", "bcryptjs": "^2.4.3",
"bits-ui": "^1.3.0", "bits-ui": "^1.3.0",
"clsx": "^2.1.0", "clsx": "^2.1.0",
"isomorphic-dompurify": "^3.7.1",
"jsonwebtoken": "^9.0.2", "jsonwebtoken": "^9.0.2",
"lucide-svelte": "^0.469.0", "lucide-svelte": "^0.469.0",
"marked": "^17.0.5", "marked": "^17.0.5",
+43 -22
View File
@@ -14,7 +14,22 @@
let loading = $state(true); let loading = $state(true);
const iframeHeight = $derived(config.height || 300); const iframeHeight = $derived(config.height || 300);
const sandboxValue = $derived(config.sandbox || 'allow-scripts allow-same-origin'); // Default sandbox: allow-scripts only. allow-same-origin is intentionally omitted
// because combining both allows the embedded page to escape the sandbox entirely.
const sandboxValue = $derived(config.sandbox || 'allow-scripts');
// Only allow http/https URLs — block javascript:, data:, etc.
const safeUrl = $derived.by(() => {
try {
const parsed = new URL(config.url);
if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
return config.url;
}
return '';
} catch {
return '';
}
});
function handleLoad() { function handleLoad() {
loading = false; loading = false;
@@ -23,28 +38,34 @@
<div class="flex flex-col rounded-xl border border-border bg-card"> <div class="flex flex-col rounded-xl border border-border bg-card">
<div class="relative" style="height: {iframeHeight}px;"> <div class="relative" style="height: {iframeHeight}px;">
{#if loading} {#if !safeUrl}
<div class="absolute inset-0 flex items-center justify-center bg-muted/50"> <div class="flex h-full items-center justify-center text-sm text-muted-foreground">
<div class="flex items-center gap-2 text-sm text-muted-foreground"> Invalid or blocked embed URL
<svg
class="h-4 w-4 animate-spin"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
>
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" />
<path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z" />
</svg>
Loading...
</div>
</div> </div>
{:else}
{#if loading}
<div class="absolute inset-0 flex items-center justify-center bg-muted/50">
<div class="flex items-center gap-2 text-sm text-muted-foreground">
<svg
class="h-4 w-4 animate-spin"
xmlns="http://www.w3.org/2000/svg"
fill="none"
viewBox="0 0 24 24"
>
<circle class="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" stroke-width="4" />
<path class="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8V0C5.373 0 0 5.373 0 12h4z" />
</svg>
Loading...
</div>
</div>
{/if}
<iframe
src={safeUrl}
title="Embedded content"
sandbox={sandboxValue}
class="h-full w-full rounded-xl border-0"
onload={handleLoad}
></iframe>
{/if} {/if}
<iframe
src={config.url}
title="Embedded content"
sandbox={sandboxValue}
class="h-full w-full rounded-xl border-0"
onload={handleLoad}
></iframe>
</div> </div>
</div> </div>
+10 -12
View File
@@ -1,5 +1,6 @@
<script lang="ts"> <script lang="ts">
import { marked } from 'marked'; import { marked } from 'marked';
import DOMPurify from 'isomorphic-dompurify';
interface NoteConfig { interface NoteConfig {
content: string; content: string;
@@ -12,7 +13,6 @@
let { config }: Props = $props(); let { config }: Props = $props();
// Configure marked for security
marked.setOptions({ marked.setOptions({
breaks: true, breaks: true,
gfm: true gfm: true
@@ -20,24 +20,22 @@
const renderedContent = $derived.by(() => { const renderedContent = $derived.by(() => {
if (config.format === 'text') { if (config.format === 'text') {
return config.content return DOMPurify.sanitize(
.replace(/&/g, '&amp;') config.content
.replace(/</g, '&lt;') .replace(/&/g, '&amp;')
.replace(/>/g, '&gt;') .replace(/</g, '&lt;')
.replace(/\n/g, '<br>'); .replace(/>/g, '&gt;')
.replace(/\n/g, '<br>')
);
} }
// Sanitize by stripping script tags and event handlers from markdown output
const raw = marked.parse(config.content, { async: false }) as string; const raw = marked.parse(config.content, { async: false }) as string;
return raw return DOMPurify.sanitize(raw);
.replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '')
.replace(/\s*on\w+\s*=\s*"[^"]*"/gi, '')
.replace(/\s*on\w+\s*=\s*'[^']*'/gi, '');
}); });
</script> </script>
<div class="flex h-full flex-col rounded-xl border border-border bg-card p-4"> <div class="flex h-full flex-col rounded-xl border border-border bg-card p-4">
<div class="prose prose-sm prose-invert max-w-none flex-1 overflow-auto text-foreground"> <div class="prose prose-sm prose-invert max-w-none flex-1 overflow-auto text-foreground">
<!-- eslint-disable-next-line svelte/no-at-html-tags -- content is sanitized above --> <!-- eslint-disable-next-line svelte/no-at-html-tags -- sanitized with DOMPurify -->
{@html renderedContent} {@html renderedContent}
</div> </div>
</div> </div>