This is an automated email from the ASF dual-hosted git repository. ahuber pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/isis.git
The following commit(s) were added to refs/heads/master by this push: new 5def8575ed ISIS-3077: restore non-escaped output rendering for certain value types 5def8575ed is described below commit 5def8575ed74e212624a70c6b794e5ee886da7fd Author: Andi Huber <ahu...@apache.org> AuthorDate: Wed Jun 15 19:59:22 2022 +0200 ISIS-3077: restore non-escaped output rendering for certain value types - also harden LocalResourcePath, URL and Markup (value types) against XSS attacks --- .../isis/applib/value/LocalResourcePath.java | 9 ++--- .../java/org/apache/isis/applib/value/Markup.java | 6 +++- .../isis/commons/internal/base/_Strings.java | 39 ++++++++++++++++++++++ .../valuesemantics/URLValueSemantics.java | 6 +--- .../ui/components/scalars/ScalarPanelAbstract.java | 4 ++- .../components/scalars/ScalarPanelAbstract2.java | 4 ++- .../ScalarPanelTextFieldWithValueSemantics.java | 7 ++++ 7 files changed, 63 insertions(+), 12 deletions(-) diff --git a/api/applib/src/main/java/org/apache/isis/applib/value/LocalResourcePath.java b/api/applib/src/main/java/org/apache/isis/applib/value/LocalResourcePath.java index acea0bd3e3..532365b983 100644 --- a/api/applib/src/main/java/org/apache/isis/applib/value/LocalResourcePath.java +++ b/api/applib/src/main/java/org/apache/isis/applib/value/LocalResourcePath.java @@ -19,7 +19,6 @@ package org.apache.isis.applib.value; import java.io.Serializable; -import java.net.URISyntaxException; import java.util.function.UnaryOperator; import javax.inject.Named; @@ -30,6 +29,8 @@ import org.springframework.lang.Nullable; import org.apache.isis.applib.IsisModuleApplib; import org.apache.isis.applib.annotation.Value; +import org.apache.isis.commons.internal.base._Blackhole; +import org.apache.isis.commons.internal.base._Strings; import lombok.Getter; import lombok.NonNull; @@ -120,9 +121,9 @@ public final class LocalResourcePath implements Serializable { return; } try { - // used for syntax testing - new java.net.URI("http://localhost/"+path); - } catch (URISyntaxException e) { + // path syntax check + _Blackhole.consume(_Strings.toUrlWithXssGuard("http://localhost/"+path)); + } catch (IllegalArgumentException e) { throw new IllegalArgumentException(String.format("the given local path has an invalid syntax: '%s'", path), e); } } diff --git a/api/applib/src/main/java/org/apache/isis/applib/value/Markup.java b/api/applib/src/main/java/org/apache/isis/applib/value/Markup.java index c47139f5a5..ce293d3b55 100644 --- a/api/applib/src/main/java/org/apache/isis/applib/value/Markup.java +++ b/api/applib/src/main/java/org/apache/isis/applib/value/Markup.java @@ -58,7 +58,7 @@ public final class Markup implements Serializable { } public Markup(final String html) { - this.html = html!=null ? html : ""; + this.html = validate(html!=null ? html : ""); } public String asHtml() { @@ -78,6 +78,10 @@ public final class Markup implements Serializable { 255, "..."); } + private String validate(final String html) { + return _Strings.htmlNoScript(html); + } + public static final class JaxbToStringAdapter extends XmlAdapter<String, Markup> { /** diff --git a/commons/src/main/java/org/apache/isis/commons/internal/base/_Strings.java b/commons/src/main/java/org/apache/isis/commons/internal/base/_Strings.java index b0ab733df4..cafa922efa 100644 --- a/commons/src/main/java/org/apache/isis/commons/internal/base/_Strings.java +++ b/commons/src/main/java/org/apache/isis/commons/internal/base/_Strings.java @@ -21,6 +21,8 @@ package org.apache.isis.commons.internal.base; import java.io.ByteArrayOutputStream; import java.io.InputStream; import java.io.PrintStream; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -535,6 +537,43 @@ public final class _Strings { return grep(input, matcher); } + // -- XSS GUARDS + + /** + * @throws IllegalArgumentException - when an XSS attack is encountered, or the URL is not parseable + * @implNote unfortunately has potential for false positives; but shall do for now + */ + public static Optional<URL> toUrlWithXssGuard(final @Nullable String urlString) { + if(urlString==null) { + return Optional.empty(); + } + if(_Strings.condenseWhitespaces(urlString.toLowerCase(), "").contains("javascript:")) { + // simple guard against XSS attacks like javascript:alert(document) + throw new IllegalArgumentException("Not parseable as an URL ('" + urlString + "')."); + } + try { + return Optional.of(new java.net.URL(urlString)); + } catch (final MalformedURLException ex) { + throw new IllegalArgumentException("Not parseable as an URL ('" + urlString + "').", ex); + } + } + + /** + * @throws IllegalArgumentException - when scripts are encountered + * @implNote unfortunately has potential for false positives; but shall do for now + */ + public static String htmlNoScript(final @Nullable String html) { + if(html==null) { + return null; + } + val condensed = _Strings.condenseWhitespaces(html.toLowerCase(), ""); + if(condensed.contains("javascript:") + || condensed.contains("<script")) { + throw new IllegalArgumentException("Not parseable as html free of scripts content."); + } + return html; + } + // -- REPLACEMENT OPERATORS diff --git a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/valuesemantics/URLValueSemantics.java b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/valuesemantics/URLValueSemantics.java index 3865c2d8a7..2d0109a923 100644 --- a/core/metamodel/src/main/java/org/apache/isis/core/metamodel/valuesemantics/URLValueSemantics.java +++ b/core/metamodel/src/main/java/org/apache/isis/core/metamodel/valuesemantics/URLValueSemantics.java @@ -107,11 +107,7 @@ implements if(input==null) { return null; } - try { - return new java.net.URL(input); - } catch (final MalformedURLException ex) { - throw new IllegalArgumentException("Not parseable as an URL ('" + input + "')", ex); - } + return _Strings.toUrlWithXssGuard(input).orElse(null); } @Override diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java index ce3ccaab5b..1f248440bd 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java +++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract.java @@ -90,7 +90,9 @@ implements ScalarModelSubscriber { COMPOSITE, TRISTATE, BLOB, - BADGE + BADGE, + /** render output un-escaped; careful not to allow XSS vulnerabilities*/ + NO_OUTPUT_ESCAPE } public enum Repaint { diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract2.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract2.java index e5426e7f0b..ecc7b4c50b 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract2.java +++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelAbstract2.java @@ -136,7 +136,9 @@ extends ScalarPanelAbstract { } return CompactFragment.LABEL .createFragment(id, this, scalarValueId-> - Wkt.label(scalarValueId, this::obtainOutputFormat)); + getFormatModifiers().contains(FormatModifier.NO_OUTPUT_ESCAPE) + ? Wkt.markup(scalarValueId, this::obtainOutputFormat) + : Wkt.label(scalarValueId, this::obtainOutputFormat)); } private boolean isUsingTextarea() { diff --git a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelTextFieldWithValueSemantics.java b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelTextFieldWithValueSemantics.java index f1b660ec42..cbe4a68af8 100644 --- a/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelTextFieldWithValueSemantics.java +++ b/viewers/wicket/ui/src/main/java/org/apache/isis/viewer/wicket/ui/components/scalars/ScalarPanelTextFieldWithValueSemantics.java @@ -18,6 +18,8 @@ */ package org.apache.isis.viewer.wicket.ui.components.scalars; +import java.util.EnumSet; + import org.apache.wicket.util.convert.IConverter; import org.apache.isis.applib.value.semantics.ValueSemanticsProvider; @@ -52,4 +54,9 @@ extends ScalarPanelTextFieldAbstract<T> { return new ConverterBasedOnValueSemantics<>(propOrParam, scalarRepresentation); } + @Override + protected void setupFormatModifiers(final EnumSet<FormatModifier> modifiers) { + modifiers.add(FormatModifier.NO_OUTPUT_ESCAPE); + } + }