You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@juneau.apache.org by ja...@apache.org on 2026/09/04 20:35:34 UTC
(juneau) branch master updated: Reject protocol-relative (and obfuscated) hrefs/srcs in isSafeMarkdownHref/isSafeImageSrc (WORK-J0516)
This is an automated email from the ASF dual-hosted git repository.
jamesbognar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/juneau.git
The following commit(s) were added to refs/heads/master by this push:
new 79ce6034fd Reject protocol-relative (and obfuscated) hrefs/srcs in isSafeMarkdownHref/isSafeImageSrc (WORK-J0516)
79ce6034fd is described below
commit 79ce6034fddab588b2d2318f74f3c50aadc362ef
Author: James Bognar <ja...@apache.org>
AuthorDate: Fri Sep 4 16:35:09 2026 -0400
Reject protocol-relative (and obfuscated) hrefs/srcs in isSafeMarkdownHref/isSafeImageSrc (WORK-J0516)
---
.../org/apache/juneau/views/juneau-views.js | 70 ++++++++++++----------
.../rest/server/views/ViewsJs_RowDetail_Test.java | 51 ++++++++++++++++
.../src/test/js/row-detail.cjs | 28 +++++++++
3 files changed, 119 insertions(+), 30 deletions(-)
diff --git a/juneau-rest/juneau-rest-server-views/src/main/resources/org/apache/juneau/views/juneau-views.js b/juneau-rest/juneau-rest-server-views/src/main/resources/org/apache/juneau/views/juneau-views.js
index faf487d412..2be28bf186 100644
--- a/juneau-rest/juneau-rest-server-views/src/main/resources/org/apache/juneau/views/juneau-views.js
+++ b/juneau-rest/juneau-rest-server-views/src/main/resources/org/apache/juneau/views/juneau-views.js
@@ -1328,22 +1328,36 @@
SCRIPT: 1, STYLE: 1, IFRAME: 1, OBJECT: 1, EMBED: 1, LINK: 1, META: 1, BASE: 1, FORM: 1, INPUT: 1, SVG: 1, IMG: 1
};
+ /**
+ * Whether `t` (already known to start with "/" or "\") is a protocol-relative URL once the obfuscation a
+ * browser's WHATWG URL/HTML parser normalizes away is stripped: ASCII tab/CR/LF removed wherever they occur
+ * (URL spec "remove all ASCII tab or newline from input"), and backslash treated as forward slash (a
+ * special-scheme URL parser folds "\" into "/" before resolving). A resulting SECOND slash right after the
+ * first -- a literal "//host", a triple-slash "///host", a two-backslash "\\host", or an interior-obfuscated
+ * "/\t/host" -- resolves to a THIRD-PARTY origin, not the same-site relative/absolute path a bare single
+ * leading "/" is. Used by both {@link #isSafeMarkdownHref} and {@link #isSafeImageSrc} -- see
+ * {@code WORK-J0516}.
+ */
+ function isProtocolRelativeUrl(t) {
+ return t.replace(/[\t\r\n]/g, "").replace(/\\/g, "/").charAt(1) === "/";
+ }
+
/**
* Whether `href` is safe to copy onto an {@code <a>}: http(s), mailto, a fragment, or a path (relative or
- * absolute). javascript:/data:/vbscript: are rejected by the explicit prefix check below, but that prefix
- * check is NOT the load-bearing defense against an obfuscated spelling of one of those schemes (e.g.
- * "java\tscript:", "da\nta:") -- a browser's URL parser strips ASCII tab/newline/CR before resolving, so a
- * literal prefix match on the un-stripped string can miss it. What actually catches that case is the final
- * fallback below: `indexOf(":") < 0`. An obfuscated scheme still contains a colon and matches none of the
- * three allowed absolute prefixes, so it still fails closed. A future "simplification" that drops this
- * fallback in favor of a longer explicit deny-list would silently reopen that hole.
+ * absolute, but NOT protocol-relative -- see below). javascript:/data:/vbscript: are rejected by the
+ * explicit prefix check below, but that prefix check is NOT the load-bearing defense against an obfuscated
+ * spelling of one of those schemes (e.g. "java\tscript:", "da\nta:") -- a browser's URL parser strips ASCII
+ * tab/newline/CR before resolving, so a literal prefix match on the un-stripped string can miss it. What
+ * actually catches that case is the final fallback below: `indexOf(":") < 0`. An obfuscated scheme still
+ * contains a colon and matches none of the three allowed absolute prefixes, so it still fails closed. A
+ * future "simplification" that drops this fallback in favor of a longer explicit deny-list would silently
+ * reopen that hole.
*
- * <p>This function does NOT reject a leading "//" -- a protocol-relative URL ("//evil.example/x") resolves
- * to a THIRD-PARTY origin, not the "relative path" this accepts it as, and the same is true of
- * backslash/tab/CR/LF-obfuscated variants ("\\evil.example", "/\t/evil.example") that a browser's HTML
- * parser normalizes to the same third-party-origin URL. That is a real, tracked gap -- phishing/open-redirect
- * severity, not script execution, since a protocol-relative URL cannot run JS -- see {@code WORK-J0516}. It
- * is intentionally not fixed here; do not read this doc as a claim that protocol-relative hrefs are rejected.
+ * <p>A leading "/" is accepted ONLY when {@link #isProtocolRelativeUrl} says it is not protocol-relative --
+ * a bare "//evil.example/x" resolves to a THIRD-PARTY origin, not the "relative path" a naive check would
+ * accept it as, and the same is true of its backslash/tab/CR/LF/triple-slash-obfuscated equivalents. This is
+ * a phishing/open-redirect defense, not an XSS one: a protocol-relative URL cannot run JS. See
+ * {@code WORK-J0516}.
*/
function isSafeMarkdownHref(href) {
if (href == null) return false;
@@ -1354,10 +1368,10 @@
return false;
if (lower.startsWith("http://") || lower.startsWith("https://") || lower.startsWith("mailto:"))
return true;
- // NOTE: a leading "/" is accepted unconditionally here, including "//evil.example" (protocol-relative,
- // third-party origin) and backslash/tab/CR/LF-obfuscated equivalents -- see the doc comment above.
- if (t.charAt(0) === "#" || t.charAt(0) === "/")
+ if (t.charAt(0) === "#")
return true;
+ if (t.charAt(0) === "/" || t.charAt(0) === "\\")
+ return !isProtocolRelativeUrl(t);
return lower.indexOf(":") < 0;
}
@@ -1527,14 +1541,20 @@
};
/**
- * Whether `src` is safe to copy onto an {@code <img>}: http(s), a path (relative or absolute -- see the
- * leading-"/" handling below), or a scheme-less relative URL. Stricter than isSafeMarkdownHref --
+ * Whether `src` is safe to copy onto an {@code <img>}: http(s), a path (relative or absolute, but NOT
+ * protocol-relative -- see below), or a scheme-less relative URL. Stricter than isSafeMarkdownHref --
* `mailto:` and bare `#fragment` are meaningless for an image, and `data:` is rejected here as it is there
* (it would also let one field inline unbounded bytes). As with isSafeMarkdownHref, the explicit
* `javascript:`/`data:`/`vbscript:` prefix check below is backed by a colon-fallback (`indexOf(":") < 0`)
* that is the actual load-bearing defense against an obfuscated spelling of one of those schemes (a
* browser's URL parser strips ASCII tab/newline/CR before resolving, so a literal prefix match on the
* un-stripped string can miss it, but an obfuscated scheme still contains a colon and still fails closed).
+ *
+ * <p>A leading "/" is accepted ONLY when {@link #isProtocolRelativeUrl} says it is not protocol-relative --
+ * a literal "//host/x", its backslash/tab/CR/LF/triple-slash-obfuscated equivalents ("\\host", "/\t/host"),
+ * and a leading-whitespace variant (stripped by `trim()` above, then caught by the same check) all resolve
+ * to a THIRD-PARTY origin, which for an {@code <img>} is a silent tracking-pixel/pixel-leak fetch. See
+ * {@code WORK-J0516}.
*/
function isSafeImageSrc(src) {
if (src == null) return false;
@@ -1545,18 +1565,8 @@
return false;
if (lower.startsWith("http://") || lower.startsWith("https://"))
return true;
- // A LITERAL leading "//" ("//host/x") is rejected here: it reads as a path but resolves to a
- // third-party origin, which for an <img> is a silent tracking-pixel/pixel-leak fetch. This is NOT a
- // comprehensive check, though: a backslash- or tab/CR/LF-obfuscated protocol-relative path ("\\host",
- // "/\t/host") does not match this literal two-character prefix and falls through to the leading-"/" or
- // scheme-less branches below, BOTH of which accept it -- a browser's HTML parser normalizes backslashes
- // to "/" and strips ASCII tab/newline/CR before resolving, so those variants resolve to the SAME
- // third-party origin this check exists to reject. That gap is real and tracked (WORK-J0516, widened to
- // cover this helper too); it is intentionally not fixed here.
- if (t.startsWith("//"))
- return false;
- if (t.charAt(0) === "/")
- return true;
+ if (t.charAt(0) === "/" || t.charAt(0) === "\\")
+ return !isProtocolRelativeUrl(t);
return t.charAt(0) !== "#" && lower.indexOf(":") < 0;
}
diff --git a/juneau-rest/juneau-rest-server-views/src/test/java/org/apache/juneau/rest/server/views/ViewsJs_RowDetail_Test.java b/juneau-rest/juneau-rest-server-views/src/test/java/org/apache/juneau/rest/server/views/ViewsJs_RowDetail_Test.java
index 9ad269f65d..6183d7ad0d 100644
--- a/juneau-rest/juneau-rest-server-views/src/test/java/org/apache/juneau/rest/server/views/ViewsJs_RowDetail_Test.java
+++ b/juneau-rest/juneau-rest-server-views/src/test/java/org/apache/juneau/rest/server/views/ViewsJs_RowDetail_Test.java
@@ -371,6 +371,30 @@ class ViewsJs_RowDetail_Test extends TestBase {
assertEquals(false, r.get("shSrc_null"));
}
+ /**
+ * {@code WORK-J0516}: {@code isSafeImageSrc} must reject protocol-relative {@code img[src]} values,
+ * including backslash/tab/CR/LF/triple-slash/leading-whitespace-obfuscated variants that the WHATWG URL
+ * parser normalizes to the same third-party origin as a literal {@code //host} - a naive
+ * {@code startsWith("//")} check alone is insufficient. A true single-leading-slash path and other
+ * legitimate srcs must not regress.
+ */
+ @Test void b17e2_imageSrcProtocolRelativeVariantsRejected() {
+ var r = report();
+ assertEquals(false, r.get("shSrc_protoRel"), "literal //host must be rejected");
+ assertEquals(false, r.get("shSrc_backslashProtoRel"),
+ "backslash-obfuscated (two literal backslashes before the host) must resolve like //host and be rejected");
+ assertEquals(false, r.get("shSrc_tabProtoRel"), "tab-obfuscated (/<TAB>/host) must be rejected");
+ assertEquals(false, r.get("shSrc_lfProtoRel"), "LF-obfuscated (/<LF>/host) must be rejected");
+ assertEquals(false, r.get("shSrc_crProtoRel"), "CR-obfuscated (/<CR>/host) must be rejected");
+ assertEquals(false, r.get("shSrc_tripleSlashProtoRel"), "triple-slash (///host) must be rejected");
+ assertEquals(false, r.get("shSrc_leadingSpaceProtoRel"), "leading-whitespace (' //host') must be rejected");
+ // Legit cases must not regress.
+ assertEquals(true, r.get("shSrc_absPath"), "a true single-leading-slash path must still be accepted");
+ assertEquals(true, r.get("shSrc_relative"), "a scheme-less relative path must still be accepted");
+ assertEquals(true, r.get("shSrc_https"));
+ assertEquals(true, r.get("shSrc_http"));
+ }
+
/**
* End-to-end through {@code fillDetailSlots}: the {@code "sanitizedHtml"} wire token must really dispatch to
* the sanitized painter. Without this, a broken dispatch would silently fall through to {@code textContent}
@@ -396,6 +420,33 @@ class ViewsJs_RowDetail_Test extends TestBase {
assertTrue(String.valueOf(r.get("title_xss")).contains("<img"));
}
+ /**
+ * {@code WORK-J0516}: {@code isSafeMarkdownHref} must reject protocol-relative {@code a[href]} values,
+ * including backslash/tab/CR/LF/triple-slash/leading-whitespace-obfuscated variants that the WHATWG URL
+ * parser normalizes to the same third-party origin as a literal {@code //host}. This is a phishing/
+ * open-redirect fix, not an XSS one - {@code javascript:} is already rejected via the colon-fallback rule
+ * exercised by {@link #b18_hrefAndTitleFill_xssSafety()}. A true single-leading-slash path, a scheme-less
+ * relative path, the allowlisted {@code mailto:} scheme, and a bare fragment must not regress - this
+ * changes existing MARKDOWN-format behavior deliberately (see the item).
+ */
+ @Test void b18a_hrefProtocolRelativeVariantsRejected() {
+ var r = report();
+ assertEquals(false, r.get("href_protoRel"), "literal //host must be rejected");
+ assertEquals(false, r.get("href_backslash"),
+ "backslash-obfuscated (two literal backslashes before the host) must resolve like //host and be rejected");
+ assertEquals(false, r.get("href_tabObfuscated"), "tab-obfuscated (/<TAB>/host) must be rejected");
+ assertEquals(false, r.get("href_lfObfuscated"), "LF-obfuscated (/<LF>/host) must be rejected");
+ assertEquals(false, r.get("href_crObfuscated"), "CR-obfuscated (/<CR>/host) must be rejected");
+ assertEquals(false, r.get("href_tripleSlash"), "triple-slash (///host) must be rejected");
+ assertEquals(false, r.get("href_leadingSpaceProtoRel"), "leading-whitespace (' //host') must be rejected");
+ // Legit cases must not regress.
+ assertEquals(true, r.get("href_singleSlashPath"), "a true single-leading-slash path must still be accepted");
+ assertEquals(true, r.get("href_relativePath"), "a scheme-less relative path must still be accepted");
+ assertEquals(true, r.get("href_mailto"), "the allowlisted mailto: scheme must still be accepted");
+ assertEquals(true, r.get("href_fragment"), "a bare fragment must still be accepted");
+ assertEquals(true, r.get("href_https"));
+ }
+
@Test void b19_paintActionMessageAndHeaderIcon() {
var r = report();
assertEquals(true, r.get("hasPaintActionMessageIntoDetail"));
diff --git a/juneau-rest/juneau-rest-server-views/src/test/js/row-detail.cjs b/juneau-rest/juneau-rest-server-views/src/test/js/row-detail.cjs
index 3241c80bf5..75545f8940 100644
--- a/juneau-rest/juneau-rest-server-views/src/test/js/row-detail.cjs
+++ b/juneau-rest/juneau-rest-server-views/src/test/js/row-detail.cjs
@@ -469,6 +469,24 @@ out.href_js = I.isSafeMarkdownHref('javascript:alert(1)');
out.href_https = I.isSafeMarkdownHref('https://x');
out.href_data = I.isSafeMarkdownHref('data:text/html,x');
+// WORK-J0516: protocol-relative hrefs -- literal and backslash/tab/CR/LF/triple-slash/leading-whitespace
+// obfuscated -- resolve to a THIRD-PARTY origin under the WHATWG URL parser, not the same-site relative path a
+// bare single leading "/" is. All of these must be rejected. '\u005C\u005C' below is two literal backslash
+// characters (the classic "\\host" bypass, equivalent to "//host").
+out.href_protoRel = I.isSafeMarkdownHref('//evil.example/x');
+out.href_backslash = I.isSafeMarkdownHref('\u005C\u005Cevil.example');
+out.href_tabObfuscated = I.isSafeMarkdownHref('/\t/evil.example');
+out.href_lfObfuscated = I.isSafeMarkdownHref('/\n/evil.example');
+out.href_crObfuscated = I.isSafeMarkdownHref('/\r/evil.example');
+out.href_tripleSlash = I.isSafeMarkdownHref('///evil.example');
+out.href_leadingSpaceProtoRel = I.isSafeMarkdownHref(' //evil.example');
+// Legit cases that must NOT regress: a true single-leading-slash path, a scheme-less relative path, the
+// allowlisted mailto: scheme, and a bare fragment.
+out.href_singleSlashPath = I.isSafeMarkdownHref('/path/to/thing');
+out.href_relativePath = I.isSafeMarkdownHref('path/to/thing');
+out.href_mailto = I.isSafeMarkdownHref('mailto:x@example.com');
+out.href_fragment = I.isSafeMarkdownHref('#frag');
+
// ----------------------------------------------------------------------------------------------------------
// SANITIZED_HTML format (DetailField.Format.SANITIZED_HTML)
// ----------------------------------------------------------------------------------------------------------
@@ -668,6 +686,16 @@ out.shSrc_mixedCaseJs = I.isSafeImageSrc('JaVaScRiPt:alert(1)');
out.shSrc_empty = I.isSafeImageSrc('');
out.shSrc_null = I.isSafeImageSrc(null);
+// WORK-J0516: same protocol-relative variant list as isSafeMarkdownHref above (literal "//" is already rejected
+// by shSrc_protoRel above -- that predates this item -- but the backslash/tab/CR/LF-obfuscated variants are not,
+// per the WORK-J0515 adversarial review that widened this item to isSafeImageSrc too).
+out.shSrc_backslashProtoRel = I.isSafeImageSrc('\u005C\u005Cevil.example');
+out.shSrc_tabProtoRel = I.isSafeImageSrc('/\t/evil.example');
+out.shSrc_lfProtoRel = I.isSafeImageSrc('/\n/evil.example');
+out.shSrc_crProtoRel = I.isSafeImageSrc('/\r/evil.example');
+out.shSrc_tripleSlashProtoRel = I.isSafeImageSrc('///evil.example');
+out.shSrc_leadingSpaceProtoRel = I.isSafeImageSrc(' //evil.example');
+
// 7. End-to-end through fillDetailSlots: the wire token "sanitizedHtml" must actually dispatch to the
// sanitized painter (not silently fall through to textContent, which would look "safe" but prove nothing).
const e2eSlot = el('div');