parser: split checking IMG and A tags, make the latter more strict
authorThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 3 Oct 2023 07:43:47 +0000 (09:43 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Tue, 3 Oct 2023 08:31:18 +0000 (10:31 +0200)
Split the logic so that each tag is handled explicitly on it's own
if-else branch, which is now safer to do as we default to
allow-only-http-like.

Also address a recently introduced regression from the implementation
of the #4756 where any user that could edit notes could use
javascript: script-urls for XSS purpose to prepare a link that could
leak private user information when another user clicked on it, at
least if they omitted basic sanity checks by looking at the URL
displayed by the browser before.

We have to override a false-positive triggered by a eslint heuristic,
a simple string compression should be always safe.

Fixes: 5cbbb9c ("fix #4756: markdown notes: allow any valid URL for a tags")
Reported-by: Hieu Dang Cong <HieuDC5@fpt.com>
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
src/Parser.js

index 0238ef0ee0bf8030eeb0529dceaecac18a09cacd..c8e1aff65080073065eb700d8868653a9adde83f 100644 (file)
@@ -35,11 +35,11 @@ Ext.define('Proxmox.Markdown', {
                    try {
                        let url = new URL(value, window.location.origin);
                        safeURL = _isHTTPLike(url.protocol);
-                       if (
-                           canonicalTagName === 'a' ||
-                           (canonicalTagName === 'img' && url.protocol.toLowerCase() === 'data:')
-                       ) {
+                       if (canonicalTagName === 'img' && url.protocol.toLowerCase() === 'data:') {
                            safeURL = true;
+                       } else if (canonicalTagName === 'a') {
+                           // allow most link protocols so admins can use short-cuts to, e.g., RDP
+                           safeURL = url.protocol.toLowerCase() !== 'javascript:'; // eslint-disable-line no-script-url
                        }
                        if (safeURL) {
                            node.attributes[i].value = url.href;