You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@isis.apache.org by ah...@apache.org on 2022/06/15 17:59:28 UTC
[isis] branch master updated: ISIS-3077: restore non-escaped output rendering for certain value types
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 <ah...@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);
+ }
+
}