fix: Serialize all String properties as sanitized HTML (#621)

This should be done for all strings except for functions to prevent
js unwanted js executions when rendering the chart

Workaround for: highcharts/highcharts#13559
See also: SNYK-JS-HIGHCHARTS-571995

addon/src/main/java/com/vaadin/addon/charts/model/serializers/StringSerializer.java

@@ -0,0 +1,76 @@
+package com.vaadin.addon.charts.model.serializers;
+
+/*-
+ * #%L
+ * Vaadin Charts Addon
+ * %%
+ * Copyright (C) 2012 - 2019 Vaadin Ltd
+ * %%
+ * This program is available under Commercial Vaadin Add-On License 3.0
+ * (CVALv3).
+ * 
+ * See the file licensing.txt distributed with this software for more
+ * information about licensing.
+ * 
+ * You should have received a copy of the CVALv3 along with this program.
+ * If not, see <https://vaadin.com/license/cval-3>.
+ * #L%
+ */
+
+import java.io.IOException;
+
+import org.jsoup.Jsoup;
+import org.jsoup.safety.Whitelist;
+
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.JsonSerializer;
+import com.fasterxml.jackson.databind.Module;
+import com.fasterxml.jackson.databind.SerializerProvider;
+import com.fasterxml.jackson.databind.module.SimpleModule;
+
+/**
+ * Serializes all {@link java.lang.String} objects that are not functions as
+ * sanitized Strings.
+ */
+public class StringSerializer extends JsonSerializer<String> {
+
+    public static Module getModule() {
+        SimpleModule module = new SimpleModule();
+        module.addSerializer(String.class, new StringSerializer());
+
+        return module;
+    }
+
+    @Override
+    public void serialize(String value, JsonGenerator gen,
+            SerializerProvider serializers)
+            throws IOException, JsonProcessingException {
+        value = shouldSanitize(value, gen) ? sanitize(value) : value;
+        gen.writeString(value);
+    }
+
+    /**
+     * Functions (_fn_ prefixed properties) and null values should not be
+     * sanitized
+     */
+    private boolean shouldSanitize(String value, JsonGenerator gen) {
+        return value != null && gen.getOutputContext().inObject()
+                && !gen.getOutputContext().getCurrentName().startsWith("_fn_");
+    }
+
+    /*
+     * Helper function for content sanitization, this preserves common
+     * formatting, but strips scripts. Workaround for:
+     * https://github.com/highcharts/highcharts/issues/13559 See also:
+     * SNYK-JS-HIGHCHARTS-571995Ï
+     */
+    private String sanitize(String html) {
+        return Jsoup.clean(html,
+                Whitelist.basic().addTags("img", "h1", "h2", "h3", "s")
+                        .addAttributes("img", "align", "alt", "height", "src",
+                                "title", "width")
+                        .addAttributes(":all", "style")
+                        .addProtocols("img", "src", "data"));
+    }
+}

addon/src/main/java/com/vaadin/addon/charts/util/ChartSerialization.java

@@ -35,6 +35,7 @@
 import com.vaadin.addon.charts.model.serializers.DefaultBeanSerializerModifier;
 import com.vaadin.addon.charts.model.serializers.GradientColorStopsSerializer;
 import com.vaadin.addon.charts.model.serializers.InstantSerializer;
+import com.vaadin.addon.charts.model.serializers.StringSerializer;
 import com.vaadin.addon.charts.model.serializers.PaneListSerializer;
 import com.vaadin.addon.charts.model.serializers.SolidColorSerializer;
 import com.vaadin.addon.charts.model.serializers.StopSerializer;
@@ -85,7 +86,8 @@ public static ObjectMapper createObjectMapper(BeanSerializerModifier modifier) {
                 .registerModule(AxisListSerializer.getModule())
                 .registerModule(PaneListSerializer.getModule())
                 .registerModule(DateSerializer.getModule())
-                .registerModule(InstantSerializer.getModule());
+                .registerModule(InstantSerializer.getModule())
+                .registerModule(StringSerializer.getModule());
 
         // serializer modifier used when basic serializer isn't enough
         return mapper.setSerializerFactory(mapper.getSerializerFactory()

addon/src/test/java/com/vaadin/addon/charts/model/junittests/UnsafeHTMLJSONSerializationTest.java

@@ -0,0 +1,37 @@
+package com.vaadin.addon.charts.model.junittests;
+
+import static com.vaadin.addon.charts.util.ChartSerialization.toJSON;
+import static org.junit.Assert.assertEquals;
+
+import org.junit.Test;
+
+import com.vaadin.addon.charts.model.Configuration;
+
+/**
+ * Tests for the sanitization of unsafe HTML in JSON serialization in
+ * {@link Configuration}
+ *
+ */
+public class UnsafeHTMLJSONSerializationTest {
+
+    @Test
+    public void configurationJSONSerialization_unsafeTitle_unsafeHTMLNotSerialized() {
+        Configuration conf = new Configuration();
+        conf.setTitle(
+                "1. JavaScript injected in plain href: <a href=\'#\";alert(\"XSS 1\");var fake=\"\'>Click here</a><br> 2. JavaScript injected as href=\"javascript:...\": <a href=\"javascript:alert(\'XSS 2\')\">Click here</a><br> Source: <a href=\"http://thebulletin.metapress.com/content/c4120650912x74k7/fulltext.pdf\">thebulletin.metapress.com</a>");
+
+        assertEquals(
+                "{\"title\":{\"text\":\"1. JavaScript injected in plain href: \\n<a rel=\\\"nofollow\\\">Click here</a>\\n<br> 2. JavaScript injected as href=\\\"javascript:...\\\": \\n<a rel=\\\"nofollow\\\">Click here</a>\\n<br> Source: \\n<a href=\\\"http://thebulletin.metapress.com/content/c4120650912x74k7/fulltext.pdf\\\" rel=\\\"nofollow\\\">thebulletin.metapress.com</a>\"},\"plotOptions\":{},\"series\":[],\"exporting\":{\"enabled\":false}}",
+                toJSON(conf));
+    }
+
+    @Test
+    public void configurationJSONSerialization_unsafeSubTitle_unsafeHTMLNotSerialized() {
+        Configuration conf = new Configuration();
+        conf.setSubTitle(
+                "1. JavaScript injected in plain href: <a href=\'#\";alert(\"XSS 1\");var fake=\"\'>Click here</a><br> 2. JavaScript injected as href=\"javascript:...\": <a href=\"javascript:alert(\'XSS 2\')\">Click here</a><br> Source: <a href=\"http://thebulletin.metapress.com/content/c4120650912x74k7/fulltext.pdf\">thebulletin.metapress.com</a>");
+        assertEquals(
+                "{\"subtitle\":{\"text\":\"1. JavaScript injected in plain href: \\n<a rel=\\\"nofollow\\\">Click here</a>\\n<br> 2. JavaScript injected as href=\\\"javascript:...\\\": \\n<a rel=\\\"nofollow\\\">Click here</a>\\n<br> Source: \\n<a href=\\\"http://thebulletin.metapress.com/content/c4120650912x74k7/fulltext.pdf\\\" rel=\\\"nofollow\\\">thebulletin.metapress.com</a>\"},\"plotOptions\":{},\"series\":[],\"exporting\":{\"enabled\":false}}",
+                toJSON(conf));
+    }
+}