summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOwen Jacobson <owen@grimoire.ca>2025-07-09 18:57:16 -0400
committerOwen Jacobson <owen@grimoire.ca>2025-07-09 21:26:10 -0400
commit01ed82ac4f89810161fbc3aa1cb8e4691fb8938b (patch)
tree15ef92a207c44d60556188b4016bafc3c85b466f
parentaf215bf98bec827fc75756b197bbb884afb52643 (diff)
Do not support users entering bare HTML in swatches.
You can inject Javascript into a swatch that uses `{@html <expr>}` fairly easily. `<script>foo()</script>` doesn't appear to work, but `<img src="x" onerror="foo()">` does, for example. That code then runs with the same access to cookies, and the same access to local data, as the Pilcrow client. This change removes that capability, by replacing the two swatches that exposed it with more limited examples. I love the generality and flexibility of generic HTML entry here, and I think it might have been useful for swatching components that are generic DOM containers (which both `Message` and `MessageRun` are today), but swatches are a user interface and are exposed to _all_ users. A user who is unfamiliar with HTML and Javascript, but who is persuaded to open a swatch and enter some code into it (think about an attacker who tells their victim "hey check out this funny thing that happens," preying on curiousity, while providing a lightly-obfuscated payload) can then impersonate that user, exfiltrate anything saved locally, or potentially install persistent code using JS' various background-processing APIs. Gnarly stuff. We're not up to mitigating that in place. Anyone who knows JS can likely learn to build the client from source, and can experiment with arbitrary input that way, taking responsibility for the results in the process, while anyone who doesn't is unlikely to be persuaded to set up an entire Node toolchain just for an exploit.
-rw-r--r--docs/developer/client/swatches.md10
-rw-r--r--ui/routes/(swatch)/.swatch/Message/+page.svelte38
-rw-r--r--ui/routes/(swatch)/.swatch/MessageRun/+page.svelte47
3 files changed, 79 insertions, 16 deletions
diff --git a/docs/developer/client/swatches.md b/docs/developer/client/swatches.md
index df4f643..ad11381 100644
--- a/docs/developer/client/swatches.md
+++ b/docs/developer/client/swatches.md
@@ -14,3 +14,13 @@ Things to consider:
- For freeform values whose meaning is significant, provide buttons to let the user rapidly enter and experiment with interesting values.
- Be thorough. Let the user experiment with values, even if you think those values may be nonsensical.
- Try to show the component _without_ supporting markup, as far as is possible. However, if a component generates markup that requires context - a table row component needs a table, for example, or a list element needs a list - then include that markup in the swatch.
+
+## Swatches and XSS
+
+⚠ Swatches **should not** accept arbitrary HTML as user input and render it. Use something less expressive, instead.
+
+Swatches are an interface _intended for_ developers, but _available to_ everyone - including users who may not be familiar with the Web platform or the level of access a page might have to their credentials or data. Putting HTML inputs on a swatch invites the risk that a user may be mislead into running code in that swatch that then has access to the Pilcrow API, or to their locally-stored data.
+
+If a component takes HTML as an argument, then the best compromise we've found between allowing a swatch user to experiment with that HTML and protecting that user from these risks is to provide some kind of structured input. For example, a swatch for a message might instead allow test data to be entered using the same Markdown dialect real messages are entered with.
+
+For components where there isn't an easy way to do that, providing one or more predetermined test bodies is also a workable alternative.
diff --git a/ui/routes/(swatch)/.swatch/Message/+page.svelte b/ui/routes/(swatch)/.swatch/Message/+page.svelte
index 6fd9b6b..6faf3bc 100644
--- a/ui/routes/(swatch)/.swatch/Message/+page.svelte
+++ b/ui/routes/(swatch)/.swatch/Message/+page.svelte
@@ -2,6 +2,7 @@
import { DateTime } from 'luxon';
import EventCapture from '$lib/swatch/event-capture.svelte.js';
+ import { render } from '$lib/markdown.js';
import Message from '$lib/components/Message.svelte';
import EventLog from '$lib/components/swatch/EventLog.svelte';
@@ -11,10 +12,37 @@
// Astonishingly, `DateTime.fromISO` does not throw on invalid inputs. It generates an "Invalid
// DateTime" sentinel value, instead.
let at = $derived(DateTime.fromISO(atInput));
- let renderedBody = $state(
- '<p>Lorem ipsum <code>dolor</code> sit amet, consectetur adipiscing elit. Nunc quis ante ac leo tristique iaculis vel in tortor. Praesent sed interdum ipsum. Pellentesque blandit, sapien at mattis facilisis, leo mi gravida erat, in euismod mi lectus non dui. Praesent at justo vel mauris pulvinar sodales ut sed nisl. Aliquam aliquet justo vel cursus imperdiet. Suspendisse potenti. Duis varius tortor finibus, rutrum justo ac, tincidunt enim.</p>\n' +
- '<p>Donec velit dui, bibendum a augue sit amet, tempus condimentum neque. Integer nibh tortor, imperdiet at aliquet eu, rutrum eget ligula. Donec porttitor nisi lacus, eu bibendum augue maximus eget. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Maecenas in est eget lectus dapibus tincidunt. Ut ut nisi egestas, posuere libero laoreet, venenatis erat. Nulla maximus, nisl eget interdum ornare, enim turpis semper ligula, sed ultricies sem sem quis arcu. Ut a dapibus augue. Pellentesque nec tincidunt sem.</p>',
+ let renderedBodyInput = $state(
+ `Lorem ipsum \`dolor\` sit amet, consectetur adipiscing elit. Nunc quis ante ac leo tristique
+iaculis vel in tortor. Praesent sed interdum ipsum. Pellentesque blandit, sapien at mattis
+facilisis, leo mi gravida erat, in euismod mi lectus non dui. Praesent at justo vel mauris pulvinar
+sodales ut sed nisl. Aliquam aliquet justo vel cursus imperdiet. Suspendisse potenti. Duis varius
+tortor finibus, rutrum justo ac, tincidunt enim.
+
+Donec velit dui, bibendum a augue sit amet, tempus condimentum neque. Integer nibh tortor, imperdiet
+at aliquet eu, rutrum eget ligula. Donec porttitor nisi lacus, eu bibendum augue maximus eget. Class
+aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Maecenas in
+est eget lectus dapibus tincidunt. Ut ut nisi egestas, posuere libero laoreet, venenatis erat. Nulla
+maximus, nisl eget interdum ornare, enim turpis semper ligula, sed ultricies sem sem quis arcu. Ut a
+dapibus augue. Pellentesque nec tincidunt sem.
+`,
);
+ /*
+ * Even though `Message` is notionally a generic container for markup, we restrict the swatch to
+ * message-flavoured Markdown. Swatches are available to all users, including
+ * technically-unsophisticated ones, and anything rendered in a swatch runs in the same origin
+ * context and the same cookie context as the rest of the client.
+ *
+ * This makes it possible that a user would be persuaded to enter something into a swatch that
+ * then runs _as them_, interacting with Pilcrow via its API or accessing client-stored data.
+ *
+ * As a proof of concept, `<img src="x" onerror="console.log('oh no')">` should not run the log
+ * statement. With generic HTML entry, it would do so. With our markdown processing, it does not
+ * (the `onerror` attribute is removed). Similarly, `script` elements are prohibited.
+ *
+ * Users who want to experiment with free HTML are encouraged to edit the swatch for themselves.
+ */
+ let renderedBody = $derived(render(renderedBodyInput));
let editable = $state(true);
let cssClass = $state('');
@@ -47,8 +75,8 @@
<label>editable <input type="checkbox" bind:checked={editable} /></label>
<label
- ><p>rendered body (html)</p>
- <textarea class="html" bind:value={renderedBody}></textarea>
+ ><p>rendered body (markdown)</p>
+ <textarea class="html" bind:value={renderedBodyInput}></textarea>
</label>
</div>
diff --git a/ui/routes/(swatch)/.swatch/MessageRun/+page.svelte b/ui/routes/(swatch)/.swatch/MessageRun/+page.svelte
index a8c8853..34118ec 100644
--- a/ui/routes/(swatch)/.swatch/MessageRun/+page.svelte
+++ b/ui/routes/(swatch)/.swatch/MessageRun/+page.svelte
@@ -1,12 +1,41 @@
<script>
+ import { DateTime } from 'luxon';
+
+ import { render } from '$lib/markdown.js';
+
import MessageRun from '$lib/components/MessageRun.svelte';
+ import Message from '$lib/components/Message.svelte';
let sender = $state('wlonk');
let cssClass = $state('own-message');
- let children = $state(
- '<p>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nunc quis ante ac leo tristique iaculis vel in tortor. Praesent sed interdum ipsum. Pellentesque blandit, sapien at mattis facilisis, leo mi gravida erat, in euismod mi lectus non dui. Praesent at justo vel mauris pulvinar sodales ut sed nisl. Aliquam aliquet justo vel cursus imperdiet. Suspendisse potenti. Duis varius tortor finibus, rutrum justo ac, tincidunt enim.</p>\n' +
- '<p>Donec velit dui, bibendum a augue sit amet, tempus condimentum neque. Integer nibh tortor, imperdiet at aliquet eu, rutrum eget ligula. Donec porttitor nisi lacus, eu bibendum augue maximus eget. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos. Maecenas in est eget lectus dapibus tincidunt. Ut ut nisi egestas, posuere libero laoreet, venenatis erat. Nulla maximus, nisl eget interdum ornare, enim turpis semper ligula, sed ultricies sem sem quis arcu. Ut a dapibus augue. Pellentesque nec tincidunt sem.</p>',
- );
+
+ /*
+ * Even though `MessageRun` is notionally a generic container for markup, we restrict the swatch
+ * to precomosed test messages. Swatches are available to all users, including
+ * technically-unsophisticated ones, and anything rendered in a swatch runs in the same origin
+ * context and the same cookie context as the rest of the client.
+ *
+ * This makes it possible that a user would be persuaded to enter something into a swatch that
+ * then runs _as them_, interacting with Pilcrow via its API or accessing client-stored data.
+ *
+ * As a proof of concept, `<img src="x" onerror="console.log('oh no')">` should not run the log
+ * statement. With generic HTML entry, it would do so.
+ *
+ * Users who want to experiment with free HTML are encouraged to edit the swatch for themselves.
+ */
+ let messages = [
+ `Lorem ipsum \`dolor\` sit amet, consectetur adipiscing elit. Nunc quis ante ac leo tristique
+iaculis vel in tortor. Praesent sed interdum ipsum. Pellentesque blandit, sapien at mattis
+facilisis, leo mi gravida erat, in euismod mi lectus non dui. Praesent at justo vel mauris pulvinar
+sodales ut sed nisl. Aliquam aliquet justo vel cursus imperdiet. Suspendisse potenti. Duis varius
+tortor finibus, rutrum justo ac, tincidunt enim.`,
+ `Donec velit dui, bibendum a augue sit amet, tempus condimentum neque. Integer nibh tortor,
+imperdiet at aliquet eu, rutrum eget ligula. Donec porttitor nisi lacus, eu bibendum augue maximus
+eget. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos himenaeos.
+Maecenas in est eget lectus dapibus tincidunt. Ut ut nisi egestas, posuere libero laoreet, venenatis
+erat. Nulla maximus, nisl eget interdum ornare, enim turpis semper ligula, sed ultricies sem sem
+quis arcu. Ut a dapibus augue. Pellentesque nec tincidunt sem.`,
+ ];
</script>
<h1><code>MessageRun</code></h1>
@@ -23,18 +52,14 @@
<button onclick={() => (cssClass = 'own-message')}>own-message</button>
<button onclick={() => (cssClass = 'other-message')}>other-message</button>
</div>
-
- <label
- ><p>children (html)</p>
- <textarea class="html" bind:value={children}></textarea>
- </label>
</div>
<h2>rendered</h2>
<div class="component-preview">
<MessageRun {sender} class={cssClass}>
- <!-- eslint-disable-next-line svelte/no-at-html-tags -->
- {@html children}
+ {#each messages.entries() as [index, message]}
+ <Message id="Mplaceholder-{index}" at={DateTime.now()} renderedBody={render(message)} />
+ {/each}
</MessageRun>
</div>