Skip to content

Commit d9a1988

Browse files
fix(security): validate + sandbox the legacy-admin iframe (#665)
legacy_url from the form-spec legacy-iframe fallback is now validated before reaching the <iframe src> / <a href> sinks: only a same-origin http(s) URL is framed/linked (new safeLegacyUrl helper, mirroring action-redirect.ts); a javascript:/data:/blob: scheme or off-origin target renders an inert error card. The iframe carries sandbox="allow-forms allow-scripts allow-same-origin". SECURITY.md §QSEC-03 adds frame-src 'self' and documents the X-Frame-Options ↔ legacy-iframe interaction. Tests cover the validator and the rejection paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 75d324b commit d9a1988

5 files changed

Lines changed: 167 additions & 2 deletions

File tree

SECURITY.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,7 @@ Content-Security-Policy:
275275
connect-src 'self'; # the API is same-origin
276276
manifest-src 'self';
277277
worker-src 'self'; # the PWA service worker
278+
frame-src 'self'; # legacy-admin iframe fallback (#659) — same-origin only
278279
frame-ancestors 'none'; # clickjacking (with X_FRAME_OPTIONS)
279280
base-uri 'self';
280281
form-action 'self';
@@ -291,6 +292,26 @@ Caveats — **validate before enforcing**:
291292
`style` attributes at runtime; it is far lower-risk than allowing
292293
inline scripts. Drop it if you verify your build needs no inline
293294
styles.
295+
- `frame-src 'self'` and the **X-Frame-Options interaction** — the SPA's
296+
legacy-admin fallback (#659) embeds the legacy admin change/add page in
297+
a **same-origin** `<iframe>` when a `ModelAdmin` overrides
298+
`change_form_template` / `add_form_template`. `frame-src 'self'` is the
299+
explicit allowlist for that frame: it both **permits** the intended
300+
same-origin embed and **blocks** an off-origin `legacy_url` (defence in
301+
depth — the SPA also validates the URL client-side as same-origin
302+
http(s) and sandboxes the iframe with `allow-forms allow-scripts
303+
allow-same-origin`). Two interaction notes:
304+
- **`X_FRAME_OPTIONS = "DENY"` will break the iframe** if it is applied
305+
to the legacy admin's *own* responses, because the legacy admin page
306+
is what gets framed. If you use the legacy-iframe fallback, scope
307+
`X-Frame-Options` (and any `frame-ancestors`) so the legacy admin
308+
permits same-origin framing — e.g. set `X_FRAME_OPTIONS = "SAMEORIGIN"`
309+
for the legacy admin responses, or omit the header on that mount.
310+
`frame-ancestors 'none'` on the *SPA shell* is fine — it controls who
311+
may frame the SPA, not what the SPA may frame.
312+
- If you do **not** use a custom `change_form_template` (the SPA renders
313+
every form from the JSON form-spec), no iframe is ever created and you
314+
can drop `frame-src` and keep `X_FRAME_OPTIONS = "DENY"` everywhere.
294315
- This policy assumes the package is mounted on its own path under your
295316
domain. If you already ship a project-wide CSP, **merge** these
296317
directives rather than replacing your policy.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { safeLegacyUrl } from './legacy-url';
4+
5+
const ORIGIN = 'https://admin.example.com';
6+
7+
describe('safeLegacyUrl (#665)', () => {
8+
it('accepts a same-origin relative legacy-admin path and returns an absolute same-origin URL', () => {
9+
expect(safeLegacyUrl({ url: '/admin/auth/group/1/change/', currentOrigin: ORIGIN })).toBe(
10+
'https://admin.example.com/admin/auth/group/1/change/',
11+
);
12+
});
13+
14+
it('accepts an absolute same-origin http(s) URL', () => {
15+
expect(
16+
safeLegacyUrl({ url: 'https://admin.example.com/admin/x/', currentOrigin: ORIGIN }),
17+
).toBe('https://admin.example.com/admin/x/');
18+
});
19+
20+
it('rejects a javascript: URL (anchor-click code execution)', () => {
21+
expect(safeLegacyUrl({ url: "javascript:alert(1)", currentOrigin: ORIGIN })).toBeNull();
22+
});
23+
24+
it('rejects data: and blob: schemes', () => {
25+
expect(safeLegacyUrl({ url: 'data:text/html,<h1>x', currentOrigin: ORIGIN })).toBeNull();
26+
expect(safeLegacyUrl({ url: 'blob:https://admin.example.com/abc', currentOrigin: ORIGIN })).toBeNull();
27+
});
28+
29+
it('rejects an off-origin http(s) URL (phishing surface inside admin chrome)', () => {
30+
expect(safeLegacyUrl({ url: 'https://attacker.example/', currentOrigin: ORIGIN })).toBeNull();
31+
expect(safeLegacyUrl({ url: '//attacker.example/x', currentOrigin: ORIGIN })).toBeNull();
32+
});
33+
34+
it('rejects an unparseable URL', () => {
35+
expect(safeLegacyUrl({ url: 'http://[::bad', currentOrigin: ORIGIN })).toBeNull();
36+
});
37+
});
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// Legacy-iframe URL validation (#665).
2+
//
3+
// The form-spec endpoint can return `{renderer: "legacy-iframe", legacy_url}`
4+
// to embed the legacy admin change/add page inside the SPA shell (#659). That
5+
// URL flows into both an `<iframe src>` and an `<a href target=_blank>`, so it
6+
// reaches two navigational sinks. The SPA *should* be able to trust the API,
7+
// but every other navigational sink in this codebase validates before using a
8+
// server-supplied URL (`action-redirect.ts` parses + origin/mount-checks the
9+
// action redirect; `views.py` percent-encodes `next`). This is the one
10+
// server-emitted URL that previously reached `src`/`href` unchecked — a
11+
// compromised, buggy, or request-influenced backend could emit a
12+
// `javascript:` URL (executes from the anchor on click) or an off-origin
13+
// `http://attacker/` URL (renders attacker content inside the authenticated
14+
// admin chrome — a high-fidelity phishing surface). We close that with the
15+
// same parse-and-validate discipline used elsewhere.
16+
//
17+
// The legacy admin lives on the SAME origin as the SPA (under its own admin
18+
// prefix, NOT the SPA mount), so the rule is: parse against the current
19+
// origin, require an `http:`/`https:` scheme, and require the parsed origin to
20+
// equal the current origin. Anything else (a non-http(s) scheme like
21+
// `javascript:`/`data:`/`blob:`, or a cross-origin target) is rejected and the
22+
// caller renders an inert error card instead of framing/linking it.
23+
24+
export interface ValidateLegacyUrlArgs {
25+
url: string;
26+
/** Current page origin. Defaults to `window.location.origin`; the test
27+
* injects a known value (jsdom's origin is fixed). */
28+
currentOrigin?: string;
29+
}
30+
31+
/**
32+
* Return a safe, same-origin http(s) URL string, or `null` when the URL is
33+
* unparseable, uses a non-http(s) scheme, or points off-origin.
34+
*
35+
* Returning the *re-serialised* parsed URL (not the raw input) means the value
36+
* handed to `src`/`href` is exactly what passed validation — no room for a
37+
* parser-differential between the check and the sink.
38+
*/
39+
export function safeLegacyUrl(args: ValidateLegacyUrlArgs): string | null {
40+
const origin = args.currentOrigin ?? window.location.origin;
41+
let parsed: URL;
42+
try {
43+
parsed = new URL(args.url, origin);
44+
} catch {
45+
return null;
46+
}
47+
// Reject `javascript:` / `data:` / `blob:` / `mailto:` etc. outright — only
48+
// real navigable web schemes are allowed in an iframe/anchor here.
49+
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return null;
50+
// The legacy admin is same-origin; an off-origin target is never legitimate.
51+
if (parsed.origin !== origin) return null;
52+
return parsed.href;
53+
}

frontend/apps/web/src/pages/detail/ChangeForm.test.tsx

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,31 @@ describe('ChangeForm (#659)', () => {
6363
const iframe = screen.getByTitle('Legacy admin form') as HTMLIFrameElement;
6464
expect(iframe).toBeInTheDocument();
6565
expect(iframe.src).toContain('/admin/auth/group/1/change/');
66+
// #665: the iframe is sandboxed with an explicit allowlist (no
67+
// allow-top-navigation / allow-popups / allow-modals).
68+
expect(iframe.getAttribute('sandbox')).toBe('allow-forms allow-scripts allow-same-origin');
69+
});
70+
71+
it('rejects a javascript: legacy_url and renders an inert error card (no iframe) (#665)', () => {
72+
specState = {
73+
data: { renderer: 'legacy-iframe', legacy_url: 'javascript:fetch("/admin/")' },
74+
loading: false,
75+
error: null,
76+
};
77+
renderChangeForm();
78+
expect(screen.queryByTitle('Legacy admin form')).not.toBeInTheDocument();
79+
expect(screen.getByText(/cant be displayed/i)).toBeInTheDocument();
80+
});
81+
82+
it('rejects an off-origin legacy_url and renders the error card (#665)', () => {
83+
specState = {
84+
data: { renderer: 'legacy-iframe', legacy_url: 'https://attacker.example/admin/' },
85+
loading: false,
86+
error: null,
87+
};
88+
renderChangeForm();
89+
expect(screen.queryByTitle('Legacy admin form')).not.toBeInTheDocument();
90+
expect(screen.getByText(/cant be displayed/i)).toBeInTheDocument();
6691
});
6792

6893
it('renders the form-spec fields (request-aware get_form / fieldsets) via EditForm', () => {

frontend/apps/web/src/pages/detail/LegacyIframe.tsx

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,35 @@ import { ExternalLink } from 'lucide-react';
1212

1313
import { Button, Card, t } from '@dar/ui';
1414

15+
import { safeLegacyUrl } from '../../legacy-url';
16+
1517
export interface LegacyIframeProps {
1618
url: string;
1719
onCancel: () => void;
1820
}
1921

2022
export function LegacyIframe({ url, onCancel }: LegacyIframeProps) {
23+
// Validate the server-supplied `legacy_url` before it reaches the iframe
24+
// `src` / anchor `href` (#665): same-origin + http(s) only. A
25+
// `javascript:` / `data:` URL or an off-origin target is rejected and we
26+
// render an inert error card instead — never frame/link an unvalidated URL.
27+
const safe = safeLegacyUrl({ url });
28+
29+
if (safe === null) {
30+
return (
31+
<Card>
32+
<div className="space-y-3">
33+
<p className="text-sm text-red-700">
34+
{t('This form can’t be displayed: its address is invalid or points off-site.')}
35+
</p>
36+
<Button type="button" variant="ghost" onClick={onCancel}>
37+
{t('Cancel')}
38+
</Button>
39+
</div>
40+
</Card>
41+
);
42+
}
43+
2144
return (
2245
<Card>
2346
<div className="space-y-3">
@@ -27,7 +50,7 @@ export function LegacyIframe({ url, onCancel }: LegacyIframeProps) {
2750
</p>
2851
<div className="flex shrink-0 items-center gap-2">
2952
<a
30-
href={url}
53+
href={safe}
3154
target="_blank"
3255
rel="noopener noreferrer"
3356
className="inline-flex items-center gap-1.5 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-sm font-medium text-gray-700 hover:bg-gray-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500"
@@ -40,8 +63,14 @@ export function LegacyIframe({ url, onCancel }: LegacyIframeProps) {
4063
</div>
4164
</div>
4265
<iframe
43-
src={url}
66+
src={safe}
4467
title={t('Legacy admin form')}
68+
// Defence-in-depth (#665): the legacy admin needs forms + scripts +
69+
// same-origin cookies to function, but this explicit allowlist drops
70+
// the unsafe-by-default capabilities — `allow-top-navigation`,
71+
// `allow-popups`, `allow-modals` — so a framed page can't break out
72+
// of the admin chrome even if `legacy_url` were ever subverted.
73+
sandbox="allow-forms allow-scripts allow-same-origin"
4574
className="h-[70vh] w-full rounded border border-gray-200 bg-white"
4675
/>
4776
</div>

0 commit comments

Comments
 (0)