diff --git a/CHANGELOG.md b/CHANGELOG.md index e0b9c04..f21281b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,55 @@ Changelog ist die Upgrade-Anleitung für die Tools. ## [Unreleased] +### basicbar-integrations (→ wird `integrations/v0.2.1`) + +**Sicherheitsfix:** `clean_media_url` prüfte den rohen String, sodass +Pfade wie `/media/%2e%2e/api/whoami/` oder `/media/a\..\b` durchkamen — +Browser lösen die enthaltenen `..`-Segmente nach dem Decodieren aber +trotzdem auf, sodass ein admin-verfasstes `` einen same-origin GET +auf beliebige Pfade auslösen konnte (basicbar#6). Geprüft wird jetzt der +decodierte, normalisierte Pfad; einfach- und doppelt-encodierte +`..`-Segmente sowie Backslashes werden abgelehnt. + +**Fix Runde 2 (basicbar#6):** drei weitere Bypasses, die der WHATWG-URL- +Parser aber ein reiner Substring-Check nicht sieht, sind jetzt ebenfalls +abgedeckt: rohe oder `%09`/`%0a`/`%0d`-codierte C0-/DEL-Steuerzeichen +(Browser entfernen sie überall im URL vor dem Auflösen, sodass z. B. +`/media/. ./api/` sonst als `/api/` aufgelöst würde); ein `?`/`#` +(roh oder codiert), das ein `..` davor verbirgt (`/media/..?x` → +`/?x`); und `url[:300]`-Truncation *nach* der Validierung, die einen +validierten langen Pfad nachträglich wieder auf eine Traversal kürzen +konnte — über 300 Zeichen wird jetzt komplett abgelehnt statt gekürzt. + +Migration: keine — reiner Bugfix, die öffentliche Signatur von +`clean_media_url`/`clean_html` ändert sich nicht. + +### @basicbar/ui (→ wird `ui/v0.5.0`) + +**Alt-Text für Bilder in `RichTextEditor`** (basicbar#7, WCAG 1.1.1): direkt +nach einem erfolgreichen Bild-Upload fragt der Editor per `window.prompt` +nach einer Beschreibung (`t("Image description (alt text)")`) — leere +Eingabe oder Abbrechen setzen beide `alt=""` (bewusst dekoratives Bild), das +Bild wird in jedem Fall eingefügt. Neuer Toolbar-Button +`t("Image description")` (nur sichtbar, wenn `onUploadImage` gesetzt ist), +deaktiviert, solange kein Bild markiert ist; bei markiertem Bild öffnet er +denselben Prompt vorausgefüllt mit dem aktuellen Alt-Text und übernimmt +Änderungen per `updateAttributes("image", { alt })` — Abbrechen lässt den +bestehenden Alt-Text unangetastet. `ToolbarButton` hat dafür eine neue +optionale `disabled`-Prop (reduzierte Deckkraft, `aria-disabled`). + +Nebenbei behoben: `useEditor` setzt jetzt `shouldRerenderOnTransaction: true` +— TipTap 3 rendert standardmäßig nicht mehr bei reinen Selektionsänderungen +neu, sodass sämtliche Toolbar-Buttons (Fett/Kursiv/Überschriften/Link und +jetzt auch der neue Bildbeschreibungs-Button), die ihren aktiven/deaktivierten +Zustand aus `editor.isActive(...)` lesen, nach einem Klick ohne Dokument- +änderung (z. B. Bild ab-/anwählen) den alten Stand zeigten, bis die nächste +Bearbeitung ein Re-Render auslöste. + +Migration: keine für Tools ohne `onUploadImage` — additiv. Tools mit +Bild-Upload ergänzen die neuen Übersetzungs-Keys +`"Image description (alt text)"` und `"Image description"`. + ### @basicbar/ui (→ wird `ui/v0.4.0`) **`RichTextEditor` + `RichText`** (modulierbar#5), aus AbstimmBAR verschoben: diff --git a/packages/django/basicbar-integrations/basicbar_integrations/html_sanitize.py b/packages/django/basicbar-integrations/basicbar_integrations/html_sanitize.py index f5088ef..58abb91 100644 --- a/packages/django/basicbar-integrations/basicbar_integrations/html_sanitize.py +++ b/packages/django/basicbar-integrations/basicbar_integrations/html_sanitize.py @@ -10,6 +10,9 @@ point anywhere (rel="noopener" is forced); images must come from the app's own /media/ storage. """ +import posixpath +from urllib.parse import unquote + import nh3 ALLOWED_TAGS = { @@ -20,13 +23,50 @@ ALLOWED_URL_SCHEMES = {"http", "https", "mailto"} +_CONTROL_CHARS = frozenset(chr(c) for c in range(0x20)) | {"\x7f"} # C0 + DEL + + +def _has_control_chars(value): + return any(c in _CONTROL_CHARS for c in value) + + def clean_media_url(url): """Accept only the app's own media storage: a relative ``/media/…`` path - without traversal — anything else becomes "".""" - url = (url or "").strip() - if url.startswith("/media/") and ".." not in url and not url.startswith("//"): - return url[:300] - return "" + whose decoded, normalised form stays inside ``/media/`` — anything else + becomes "". Percent-encoded dot segments (``%2e%2e``), double encoding + and backslashes are rejected (basicbar#6), as are C0/DEL control + characters (e.g. a raw or ``%09``-encoded tab — browsers strip these + anywhere in a URL before resolving it, so ``normpath`` must never see + them), a ``?``/``#`` query or fragment (raw or encoded — a trailing + ``..`` hidden after one is still resolved by the browser; query strings + on media URLs aren't supported), and anything over 300 characters + (truncating *after* validation could cut a long, validated path back + down to a traversal).""" + url = url or "" + if len(url) > 300: + return "" + if ( + not url.startswith("/media/") + or url.startswith("//") + or "\\" in url + or "?" in url + or "#" in url + or _has_control_chars(url) + ): + return "" + decoded = unquote(url) + if ( + "\\" in decoded + or "%" in decoded + or "?" in decoded + or "#" in decoded + or _has_control_chars(decoded) + ): + return "" + normalised = posixpath.normpath(decoded) + if normalised != "/media" and not normalised.startswith("/media/"): + return "" + return url def _attribute_filter(tag, attr, value): diff --git a/packages/django/basicbar-integrations/basicbar_integrations/tests/test_html_sanitize.py b/packages/django/basicbar-integrations/basicbar_integrations/tests/test_html_sanitize.py index ff3f894..b77c811 100644 --- a/packages/django/basicbar-integrations/basicbar_integrations/tests/test_html_sanitize.py +++ b/packages/django/basicbar-integrations/basicbar_integrations/tests/test_html_sanitize.py @@ -42,3 +42,56 @@ def test_clean_media_url_accepts_only_local_media(self): self.assertEqual(clean_media_url("https://evil/x.png"), "") self.assertEqual(clean_media_url("/media/../etc/passwd"), "") self.assertEqual(clean_media_url("//evil/x.png"), "") + + def test_clean_media_url_rejects_encoded_path_traversal(self): + for url in ( + "/media/%2e%2e/api/whoami/", + "/media/.%2E/x", + "/media/%2E%2E/x", + "/media/%252e%252e/x", + "/media/..%2fapi", + "/media/a\\..\\b", + "/media/a/../../api", + "//evil.example/media/x", + "https://evil.example/media/x", + ): + self.assertEqual(clean_media_url(url), "", url) + + def test_clean_media_url_keeps_valid_paths(self): + self.assertEqual(clean_media_url("/media/rich/abc.png"), "/media/rich/abc.png") + self.assertEqual( + clean_media_url("/media/products/Kuche2.jpeg"), "/media/products/Kuche2.jpeg" + ) + self.assertEqual(clean_media_url("/media/rich/a%20b.png"), "/media/rich/a%20b.png") + self.assertEqual(clean_media_url("/media/rich/gr%C3%BC%C3%9F.png"), "/media/rich/gr%C3%BC%C3%9F.png") + + def test_clean_html_drops_img_src_with_encoded_traversal(self): + self.assertNotIn("api", clean_html('')) + + def test_clean_media_url_rejects_control_characters(self): + for url in ( + "/media/.\t./api/", + "/media/.\n./api/", + "/media/%2e\r%2e/api/", + "/media/%09", + ): + self.assertEqual(clean_media_url(url), "", repr(url)) + + def test_clean_media_url_rejects_query_or_fragment(self): + for url in ( + "/media/..?x", + "/media/.%2e?x", + "/media/%2e%2e#f", + "/media/rich/a.png?v=1", + ): + self.assertEqual(clean_media_url(url), "", url) + + def test_clean_media_url_rejects_over_length_url_instead_of_truncating(self): + # Validated-then-truncated would cut this back down to ".../../..zzz" + # which normalises outside /media/ — must be rejected outright. + url = "/media/" + "a" * 287 + "/../..zzz" + self.assertEqual(len(url), 303) + self.assertEqual(clean_media_url(url), "") + + def test_clean_html_drops_img_src_with_encoded_control_char(self): + self.assertNotIn("api", clean_html('')) diff --git a/packages/django/basicbar-integrations/pyproject.toml b/packages/django/basicbar-integrations/pyproject.toml index 7ca6816..501723b 100644 --- a/packages/django/basicbar-integrations/pyproject.toml +++ b/packages/django/basicbar-integrations/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "basicbar-integrations" -version = "0.2.0" +version = "0.2.1" description = "Optionale Integrationen der virtUOS -bar-Tools: LibreTranslate, LiteLLM, Capabilities-Endpoint" readme = "README.md" requires-python = ">=3.12" diff --git a/packages/ui/README.md b/packages/ui/README.md index 28838eb..d5d316f 100644 --- a/packages/ui/README.md +++ b/packages/ui/README.md @@ -108,6 +108,16 @@ URL als String; scheitert der Upload, zeigt der Editor /> ``` +**Bildbeschreibung (Alt-Text, WCAG 1.1.1):** direkt nach einem erfolgreichen +Upload fragt der Editor per `window.prompt` nach einer Beschreibung +(`t("Image description (alt text)")`) — leere Eingabe oder Abbrechen setzen +beide `alt=""` (bewusst dekoratives Bild), das Bild wird in jedem Fall +eingefügt. Ein eigener Toolbar-Button (`t("Image description")`, nur +sichtbar, wenn `onUploadImage` gesetzt ist) ist deaktiviert, solange kein +Bild markiert ist; bei markiertem Bild öffnet er denselben Prompt, +vorausgefüllt mit dem aktuellen Alt-Text, und übernimmt die Änderung — +Abbrechen lässt den bestehenden Alt-Text unangetastet. + **Bilder aus eingefügtem HTML werden gefiltert, nicht nur Datei-Paste/-Drop:** Fügt man Rich-HTML aus einer Webseite ein (Browser-Copy&Paste, nicht als Datei), landet es über ProseMirrors HTML-Parser im Dokument — ein @@ -160,4 +170,5 @@ statt ein zweites, redundantes `ariaLabel` zu brauchen: siehe `initI18n`): `"Bold"`, `"Italic"`, `"Heading (large)"`, `"Heading (small)"`, `"Bulleted list"`, `"Numbered list"`, `"Link"`, `"Enter URL"`, `"Insert image (or drag and drop)"`, -`"Image upload failed"`. +`"Image upload failed"`, `"Image description (alt text)"`, +`"Image description"`. diff --git a/packages/ui/package-lock.json b/packages/ui/package-lock.json index 778b739..d58d182 100644 --- a/packages/ui/package-lock.json +++ b/packages/ui/package-lock.json @@ -1,12 +1,12 @@ { "name": "@basicbar/ui", - "version": "0.4.0", + "version": "0.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@basicbar/ui", - "version": "0.4.0", + "version": "0.5.0", "license": "Apache-2.0", "dependencies": { "@tiptap/extension-bold": "^3.27.2", @@ -33,6 +33,7 @@ "i18next-browser-languagedetector": ">=7", "lucide-react": ">=0.400", "react": ">=18", + "react-dom": ">=18", "react-i18next": ">=14" } }, diff --git a/packages/ui/package.json b/packages/ui/package.json index a8d163f..3658fd6 100644 --- a/packages/ui/package.json +++ b/packages/ui/package.json @@ -1,6 +1,6 @@ { "name": "@basicbar/ui", - "version": "0.4.0", + "version": "0.5.0", "description": "Design-System-Basis der virtUOS -bar-Tools: Tailwind-Preset, Basis-Styles, Theme (Dark Mode), i18n-Bootstrap, contentLang, RichTextEditor/RichText (TipTap)", "license": "Apache-2.0", "author": "Universität Osnabrück (virtUOS)", diff --git a/packages/ui/src/RichTextEditor.tsx b/packages/ui/src/RichTextEditor.tsx index 3b91028..b4116f1 100644 --- a/packages/ui/src/RichTextEditor.tsx +++ b/packages/ui/src/RichTextEditor.tsx @@ -18,6 +18,7 @@ import { EditorContent, useEditor, type Editor } from "@tiptap/react"; import StarterKit from "@tiptap/starter-kit"; import { Bold as BoldIcon, + Captions, Heading2, Heading3, ImagePlus, @@ -62,11 +63,13 @@ export interface RichTextEditorProps { function ToolbarButton({ active, + disabled, label, onClick, children, }: { active?: boolean; + disabled?: boolean; label: string; onClick: () => void; children: React.ReactNode; @@ -77,10 +80,17 @@ function ToolbarButton({ title={label} aria-label={label} aria-pressed={active} + // aria-disabled (not native disabled) keeps the button in the tab order, + // so keyboard users can discover it before selecting an image. + aria-disabled={disabled || undefined} onMouseDown={(event) => event.preventDefault()} - onClick={onClick} + onClick={disabled ? undefined : onClick} className={`rounded px-2 py-1 text-sm ${ - active ? "bg-brand-100 dark:bg-brand-900 text-brand-800 dark:text-brand-200" : "text-slate-600 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-slate-800" + disabled + ? "cursor-not-allowed text-slate-400 opacity-50 dark:text-slate-600" + : active + ? "bg-brand-100 dark:bg-brand-900 text-brand-800 dark:text-brand-200" + : "text-slate-600 dark:text-slate-300 hover:bg-slate-100 dark:hover:bg-slate-800" }`} > {children} @@ -108,13 +118,25 @@ export function RichTextEditor({ window.alert(`${t("Image upload failed")}${detail}`); return; } + // Ask for alt text once the upload has actually succeeded (WCAG 1.1.1); + // empty input or Cancel both mean "no description" — the image is + // inserted either way, just as a (possibly) decorative one. + const alt = (window.prompt(t("Image description (alt text)"), "") ?? "").trim(); const chain = editor.chain().focus(); - if (pos !== undefined) chain.insertContentAt(pos, { type: "image", attrs: { src: url } }); - else chain.setImage({ src: url }); + if (pos !== undefined) chain.insertContentAt(pos, { type: "image", attrs: { src: url, alt } }); + else chain.setImage({ src: url, alt }); chain.run(); } const editor = useEditor({ + // @tiptap/react v3 no longer re-renders on every transaction by default + // (perf choice — see `useEditorState`); we do want that, though: the + // toolbar reads `editor.isActive(...)` directly in the render body for + // every button (Bold/Italic/headings/link and now "Image description"), + // so a selection-only change (e.g. clicking into/out of an image, no + // document change) must still re-render this component or those buttons + // go stale until the next actual edit. + shouldRerenderOnTransaction: true, extensions: [ StarterKit.configure({ heading: { levels: [2, 3] }, @@ -240,6 +262,14 @@ export function RichTextEditor({ editor.chain().focus().extendMarkRange("link").setLink({ href: url }).run(); } + function setImageAlt() { + if (!editor || !editor.isActive("image")) return; + const previous = (editor.getAttributes("image").alt as string) ?? ""; + const alt = window.prompt(t("Image description (alt text)"), previous); + if (alt === null) return; // cancelled: leave the current alt untouched + editor.chain().focus().updateAttributes("image", { alt: alt.trim() }).run(); + } + return (
@@ -311,6 +341,15 @@ export function RichTextEditor({ /> )} + {onUploadImage && ( + + + + )}