Skip to content

Commit

Permalink
fix: __FRSH_STATE potentially being overwritten by user code
Browse files Browse the repository at this point in the history
  • Loading branch information
marvinhagemeister committed Jan 15, 2024
1 parent 3bc43a1 commit 29dcc9b
Show file tree
Hide file tree
Showing 15 changed files with 118 additions and 14 deletions.
5 changes: 3 additions & 2 deletions src/runtime/entrypoints/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ class NoPartialsError extends Error {}
*/
export async function applyPartials(res: Response): Promise<void> {
const contentType = res.headers.get("Content-Type");
const uuid = res.headers.get("X-Fresh-UUID");
if (contentType !== "text/html; charset=utf-8") {
throw new Error(partialErrorMessage);
}
Expand All @@ -652,7 +653,7 @@ export async function applyPartials(res: Response): Promise<void> {
// Preload all islands because they need to be available synchronously
// for rendering later
const islands: IslandRegistry = {};
const dataRaw = doc.getElementById("__FRSH_PARTIAL_DATA")!;
const dataRaw = doc.getElementById(`__FRSH_PARTIAL_DATA_${uuid}`)!;
let data: {
islands: Record<string, { export: string; url: string }>;
signals: string | null;
Expand All @@ -669,7 +670,7 @@ export async function applyPartials(res: Response): Promise<void> {
);
}

const stateDom = doc.getElementById("__FRSH_STATE")?.textContent;
const stateDom = doc.getElementById(`__FRSH_STATE_${uuid}`)?.textContent;
let state: SerializedState = [[], []];

// Load all dependencies
Expand Down
7 changes: 4 additions & 3 deletions src/server/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -669,7 +669,7 @@ const createRenderNotFound = (
return async (req, ctx) => {
const notFound = extractResult.notFound;
if (!notFound.component) {
return sendResponse(["Not found.", undefined], {
return sendResponse(["Not found.", "", undefined], {
status: STATUS_CODE.NotFound,
isDev: dev,
statusText: undefined,
Expand Down Expand Up @@ -771,19 +771,20 @@ function collectEntrypoints(
}

function sendResponse(
resp: [string, ContentSecurityPolicy | undefined],
resp: [string, string, ContentSecurityPolicy | undefined],
options: {
status: number;
statusText: string | undefined;
headers?: HeadersInit;
isDev: boolean;
},
) {
const [body, uuid, csp] = resp;
const headers: Record<string, string> = {
"content-type": "text/html; charset=utf-8",
"x-fresh-uuid": uuid,
};

const [body, csp] = resp;
if (csp) {
if (options.isDev) {
csp.directives.connectSrc = [
Expand Down
5 changes: 3 additions & 2 deletions src/server/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export function checkAsyncComponent<T>(
*/
export async function render<Data>(
opts: RenderOptions<Data>,
): Promise<[string, ContentSecurityPolicy | undefined] | Response> {
): Promise<[string, string, ContentSecurityPolicy | undefined] | Response> {
const component = opts.route.component;

// Only inherit layouts up to the nearest root layout.
Expand Down Expand Up @@ -242,6 +242,7 @@ export async function render<Data>(
// ensures that each render request is associated with the same
// data.
const renderState = new RenderState(
crypto.randomUUID(),
{
url,
route: opts.route.pattern,
Expand Down Expand Up @@ -393,5 +394,5 @@ export async function render<Data>(
moduleScripts: result.moduleScripts,
lang: ctx.lang,
});
return [html, csp];
return [html, renderState.renderUuid, csp];
}
12 changes: 8 additions & 4 deletions src/server/rendering/fresh_tags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,16 +72,20 @@ export function renderFreshTags(
// The inline script that will hydrate the page.
let script = "";

// Serialize the state into the <script id=__FRSH_STATE> tag and generate the
// Serialize the state into the <script id="__FRSH_STATE-<uuid>"> tag and generate the
// inline script to deserialize it. This script starts by deserializing the
// state in the tag. This potentially requires importing @preact/signals.
let hasSignals = false;
let requiresDeserializer = false;
if (state[0].length > 0 || state[1].length > 0) {
// Careful: This must be unique per render to avoid injected content
// via `dangerouslySetInnerHTML` being able to overwrite our state.
const stateId = `__FRSH_STATE_${renderState.renderUuid}`;

const res = serialize(state);
const escapedState = htmlEscapeJsonString(res.serialized);
opts.bodyHtml +=
`<script id="__FRSH_STATE" type="application/json" nonce="${renderState.getNonce()}">${escapedState}</script>`;
`<script id="${stateId}" type="application/json" nonce="${renderState.getNonce()}">${escapedState}</script>`;

hasSignals = res.hasSignals;
requiresDeserializer = res.requiresDeserializer;
Expand All @@ -94,7 +98,7 @@ export function renderFreshTags(
const url = addImport("signals.js");
script += `import { signal } from "${url}";`;
}
script += `const ST = document.getElementById("__FRSH_STATE").textContent;`;
script += `const ST = document.getElementById("${stateId}").textContent;`;
script += `const STATE = `;
if (res.requiresDeserializer) {
if (res.hasSignals) {
Expand Down Expand Up @@ -166,7 +170,7 @@ export function renderFreshTags(
);
const nonce = renderState.csp ? ` nonce="${renderState.getNonce()}` : "";
opts.bodyHtml +=
`<script id="__FRSH_PARTIAL_DATA" type="application/json"${nonce}">${escapedData}</script>`;
`<script id="__FRSH_PARTIAL_DATA_${renderState.renderUuid}" type="application/json"${nonce}">${escapedData}</script>`;
}
if (script !== "") {
opts.bodyHtml +=
Expand Down
3 changes: 3 additions & 0 deletions src/server/rendering/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export interface RenderStateRouteOptions {
}

export class RenderState {
readonly renderUuid: string;
// deno-lint-ignore no-explicit-any
componentStack: any[];
renderingUserTemplate = false;
Expand Down Expand Up @@ -48,12 +49,14 @@ export class RenderState {
basePath: string;

constructor(
renderUuid: string,
routeOptions: RenderStateRouteOptions,
// deno-lint-ignore no-explicit-any
componentStack: any[],
csp?: ContentSecurityPolicy,
error?: unknown,
) {
this.renderUuid = renderUuid;
this.routeOptions = routeOptions;
this.csp = csp;
this.componentStack = componentStack;
Expand Down
4 changes: 4 additions & 0 deletions tests/fixture/fresh.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import * as $route_groups_bar_boof_index from "./routes/route-groups/(bar)/boof/
import * as $route_groups_foo_layout from "./routes/route-groups/(foo)/_layout.tsx";
import * as $route_groups_foo_index from "./routes/route-groups/(foo)/index.tsx";
import * as $signal_shared from "./routes/signal_shared.tsx";
import * as $spoof_state from "./routes/spoof_state.tsx";
import * as $state_in_props_middleware from "./routes/state-in-props/_middleware.ts";
import * as $state_in_props_index from "./routes/state-in-props/index.tsx";
import * as $state_middleware_middleware from "./routes/state-middleware/_middleware.ts";
Expand All @@ -85,6 +86,7 @@ import * as $std from "./routes/std.tsx";
import * as $umlaut_äöüß from "./routes/umlaut-äöüß.tsx";
import * as $wildcard from "./routes/wildcard.tsx";
import * as $Counter from "./islands/Counter.tsx";
import * as $DangerousIsland from "./islands/DangerousIsland.tsx";
import * as $Foo_Bar from "./islands/Foo.Bar.tsx";
import * as $FormIsland from "./islands/FormIsland.tsx";
import * as $Greeter from "./islands/Greeter.tsx";
Expand Down Expand Up @@ -194,6 +196,7 @@ const manifest = {
"./routes/route-groups/(foo)/_layout.tsx": $route_groups_foo_layout,
"./routes/route-groups/(foo)/index.tsx": $route_groups_foo_index,
"./routes/signal_shared.tsx": $signal_shared,
"./routes/spoof_state.tsx": $spoof_state,
"./routes/state-in-props/_middleware.ts": $state_in_props_middleware,
"./routes/state-in-props/index.tsx": $state_in_props_index,
"./routes/state-middleware/_middleware.ts": $state_middleware_middleware,
Expand All @@ -208,6 +211,7 @@ const manifest = {
},
islands: {
"./islands/Counter.tsx": $Counter,
"./islands/DangerousIsland.tsx": $DangerousIsland,
"./islands/Foo.Bar.tsx": $Foo_Bar,
"./islands/FormIsland.tsx": $FormIsland,
"./islands/Greeter.tsx": $Greeter,
Expand Down
10 changes: 10 additions & 0 deletions tests/fixture/islands/DangerousIsland.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useEffect, useState } from "preact/hooks";

export default function RawIsland(props: { raw: string }) {
const [css, set] = useState("");
useEffect(() => {
set("raw_ready");
}, []);

return <div class={css} dangerouslySetInnerHTML={{ __html: props.raw }} />;
}
5 changes: 5 additions & 0 deletions tests/fixture/routes/spoof_state.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import DangerousIsland from "../islands/DangerousIsland.tsx";

export default function SerializePrototype() {
return <DangerousIsland raw={`<h1 id="__FRSH_STATE">{.invalid.json}</h1>`} />;
}
6 changes: 6 additions & 0 deletions tests/fixture_partials/fresh.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,12 @@ import * as $relative_link_index from "./routes/relative_link/index.tsx";
import * as $scroll_restoration_index from "./routes/scroll_restoration/index.tsx";
import * as $scroll_restoration_injected from "./routes/scroll_restoration/injected.tsx";
import * as $scroll_restoration_update from "./routes/scroll_restoration/update.tsx";
import * as $spoof_state_index from "./routes/spoof_state/index.tsx";
import * as $spoof_state_partial from "./routes/spoof_state/partial.tsx";
import * as $Counter from "./islands/Counter.tsx";
import * as $CounterA from "./islands/CounterA.tsx";
import * as $CounterB from "./islands/CounterB.tsx";
import * as $DangerousIsland from "./islands/DangerousIsland.tsx";
import * as $Fader from "./islands/Fader.tsx";
import * as $InvalidSlot from "./islands/InvalidSlot.tsx";
import * as $KeyExplorer from "./islands/KeyExplorer.tsx";
Expand Down Expand Up @@ -262,11 +265,14 @@ const manifest = {
"./routes/scroll_restoration/index.tsx": $scroll_restoration_index,
"./routes/scroll_restoration/injected.tsx": $scroll_restoration_injected,
"./routes/scroll_restoration/update.tsx": $scroll_restoration_update,
"./routes/spoof_state/index.tsx": $spoof_state_index,
"./routes/spoof_state/partial.tsx": $spoof_state_partial,
},
islands: {
"./islands/Counter.tsx": $Counter,
"./islands/CounterA.tsx": $CounterA,
"./islands/CounterB.tsx": $CounterB,
"./islands/DangerousIsland.tsx": $DangerousIsland,
"./islands/Fader.tsx": $Fader,
"./islands/InvalidSlot.tsx": $InvalidSlot,
"./islands/KeyExplorer.tsx": $KeyExplorer,
Expand Down
10 changes: 10 additions & 0 deletions tests/fixture_partials/islands/DangerousIsland.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { useEffect, useState } from "preact/hooks";

export default function DangerousIsland(props: { raw: string }) {
const [css, set] = useState("");
useEffect(() => {
set("raw_ready");
}, []);

return <div class={css} dangerouslySetInnerHTML={{ __html: props.raw }} />;
}
12 changes: 12 additions & 0 deletions tests/fixture_partials/routes/spoof_state/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Partial } from "$fresh/runtime.ts";

export default function SerializePrototype() {
return (
<div>
<Partial name="content">
<p>initial</p>
</Partial>
<a href="/spoof_state/partial">Update</a>
</div>
);
}
11 changes: 11 additions & 0 deletions tests/fixture_partials/routes/spoof_state/partial.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Partial } from "$fresh/runtime.ts";
import DangerousIsland from "../../islands/DangerousIsland.tsx";

export default function Res() {
return (
<Partial name="content">
<DangerousIsland raw={`<h1 id="__FRSH_STATE">{.invalid.json}</h1>`} />
<p class="done">partial</p>
</Partial>
);
}
18 changes: 17 additions & 1 deletion tests/main_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
withFakeServe,
withPageName,
} from "./test_utils.ts";
import { composeMiddlewares } from "$fresh/src/server/compose.ts";

const ctx = await ServerContext.fromManifest(manifest, config);
const handler = ctx.handler();
Expand Down Expand Up @@ -949,7 +950,7 @@ Deno.test("Adds nonce to inline scripts", async () => {
await withFakeServe("./tests/fixture/main.ts", async (server) => {
const doc = await server.getHtml(`/nonce_inline`);

const stateScript = doc.querySelector("#__FRSH_STATE")!;
const stateScript = doc.querySelector("[id^=__FRSH_STATE]")!;
const nonce = stateScript.getAttribute("nonce")!;

const el = doc.querySelector("#inline-script")!;
Expand Down Expand Up @@ -1220,3 +1221,18 @@ Deno.test("empty string fallback for optional params", async () => {
assertEquals(data, { path: "foo", version: "" });
});
});

// See https://github.com/denoland/fresh/issues/2254
Deno.test("should not be able to override __FRSH_STATE", async () => {
await withPageName("./tests/fixture/main.ts", async (page, address) => {
let didError = false;
page.on("pageerror", (ev) => {
didError = true;
console.log(ev);
});
await page.goto(`${address}/spoof_state`);
await page.waitForSelector(".raw_ready");

assert(!didError);
});
});
20 changes: 20 additions & 0 deletions tests/partials_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1569,3 +1569,23 @@ Deno.test("render partial without title", async () => {
},
);
});

// See https://github.com/denoland/fresh/issues/2254
Deno.test("should not be able to override __FRSH_STATE", async () => {
await withPageName(
"./tests/fixture_partials/main.ts",
async (page, address) => {
let didError = false;
page.on("pageerror", (ev) => {
didError = true;
console.log(ev);
});
await page.goto(`${address}/spoof_state`);

await page.click("a");
await page.waitForSelector(".raw_ready");

assert(!didError);
},
);
});
4 changes: 2 additions & 2 deletions tests/render_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ Deno.test("doesn't leak data across renderers", async () => {
const resp = await handler(req);
const doc = parseHtml(await resp.text());

assertSelector(doc, "#__FRSH_STATE");
const text = doc.querySelector("#__FRSH_STATE")?.textContent!;
assertSelector(doc, "[id^=__FRSH_STATE]");
const text = doc.querySelector("[id^=__FRSH_STATE]")?.textContent!;
const json = JSON.parse(text);
assertEquals(json, { "v": [[{ "site": name }], []] });
}
Expand Down

0 comments on commit 29dcc9b

Please sign in to comment.