diff --git a/jspwiki-main/src/main/java/org/apache/wiki/plugin/Image.java b/jspwiki-main/src/main/java/org/apache/wiki/plugin/Image.java
index 1429b3ebdd..e8b8db047f 100644
--- a/jspwiki-main/src/main/java/org/apache/wiki/plugin/Image.java
+++ b/jspwiki-main/src/main/java/org/apache/wiki/plugin/Image.java
@@ -32,6 +32,7 @@ Licensed to the Apache Software Foundation (ASF) under one
import java.util.Map;
import java.util.ResourceBundle;
+import java.util.Set;
/**
@@ -84,6 +85,16 @@ public class Image implements Plugin {
/** The parameter name for setting the title. Value is {@value}. */
public static final String PARAM_TITLE = "title";
+ /** Purely presentational CSS properties the style parameter may set. Repositioning and stacking
+ properties (position, z-index, transform, inset, ...) are deliberately absent, so an author cannot lift
+ the rendered box out of the page flow and overlay other content with it (in-wiki phishing overlays). */
+ private static final Set< String > SAFE_STYLE_PROPERTIES = Set.of(
+ "background", "background-color", "border", "border-color", "border-radius", "border-style",
+ "border-width", "color", "display", "float", "font-family", "font-size", "font-style", "font-weight",
+ "height", "margin", "margin-bottom", "margin-left", "margin-right", "margin-top", "max-height",
+ "max-width", "min-height", "min-width", "opacity", "padding", "padding-bottom", "padding-left",
+ "padding-right", "padding-top", "text-align", "vertical-align", "white-space", "width" );
+
/**
* This method is used to clean away things like quotation marks which
* a malicious user could use to stop processing and insert javascript.
@@ -103,6 +114,47 @@ public String getSnipExample() {
return "Image src='{image.jpg}'";
}
+ /**
+ * Accepts an author-supplied style only if every declaration sets an allow-listed presentational property.
+ * CSS escapes and functions (backslash, parentheses) and negative lengths are rejected outright: they could
+ * disguise a forbidden property (\\70 osition), smuggle a URL (url(...)), or drag the
+ * rendered box over other page content (margin-top:-9999px).
+ *
+ * @param style the author-supplied style parameter, already entity-encoded
+ * @return true if every declaration is allow-listed
+ */
+ static boolean isSafeStyle( final String style ) {
+ if( style.indexOf( '\\' ) >= 0 || style.indexOf( '(' ) >= 0 ) {
+ return false;
+ }
+ for( final String declaration : style.split( ";" ) ) {
+ if( declaration.trim().isEmpty() ) {
+ continue;
+ }
+ final int colon = declaration.indexOf( ':' );
+ if( colon < 0 ) {
+ return false;
+ }
+ final String property = declaration.substring( 0, colon ).trim().toLowerCase( Locale.ENGLISH );
+ final String value = declaration.substring( colon + 1 );
+ if( !SAFE_STYLE_PROPERTIES.contains( property ) || value.matches( ".*-\\s*\\.?\\d.*" ) ) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * Restricts the class parameter to a whitespace-separated list of CSS identifiers, so it cannot
+ * carry CSS or markup fragments.
+ *
+ * @param cssclass the author-supplied class parameter
+ * @return true if it is a plain list of identifiers
+ */
+ static boolean isSafeCssClass( final String cssclass ) {
+ return cssclass.matches( "[\\w][\\w-]*(\\s+[\\w][\\w-]*)*" );
+ }
+
private boolean needsSanitization(String link) {
String testVal = link.toLowerCase().replaceAll("\\s+", "").trim();
if (testVal.startsWith("data")
@@ -120,15 +172,15 @@ private boolean needsSanitization(String link) {
public String execute( final Context context, final Map params ) throws PluginException {
final Engine engine = context.getEngine();
String src = getCleanParameter( params, PARAM_SRC );
- final String align = getCleanParameter( params, PARAM_ALIGN );
+ String align = getCleanParameter( params, PARAM_ALIGN );
final String ht = getCleanParameter( params, PARAM_HEIGHT );
final String wt = getCleanParameter( params, PARAM_WIDTH );
final String alt = getCleanParameter( params, PARAM_ALT );
final String caption = getCleanParameter( params, PARAM_CAPTION );
String link = getCleanParameter( params, PARAM_LINK );
String target = getCleanParameter( params, PARAM_TARGET );
- final String style = getCleanParameter( params, PARAM_STYLE );
- final String cssclass= getCleanParameter( params, PARAM_CLASS );
+ String style = getCleanParameter( params, PARAM_STYLE );
+ String cssclass = getCleanParameter( params, PARAM_CLASS );
final String border = getCleanParameter( params, PARAM_BORDER );
final String title = getCleanParameter( params, PARAM_TITLE );
@@ -142,6 +194,18 @@ public String execute( final Context context, final Map params )
target = null; // not a valid value so ignore
}
+ if( align != null && !align.equals( "left" ) && !align.equals( "right" ) && !align.equals( "center" ) ) {
+ align = null; // not a valid value so ignore; it is emitted into a CSS float declaration
+ }
+
+ if( style != null && !isSafeStyle( style ) ) {
+ style = null; // only allow-listed presentational CSS may pass, see isSafeStyle()
+ }
+
+ if( cssclass != null && !isSafeCssClass( cssclass ) ) {
+ cssclass = null; // not a plain list of CSS identifiers so ignore
+ }
+
try {
final AttachmentManager mgr = engine.getManager( AttachmentManager.class );
final Attachment att = mgr.getAttachmentInfo( context, src );
diff --git a/jspwiki-main/src/test/java/org/apache/wiki/plugin/ImageStyleTest.java b/jspwiki-main/src/test/java/org/apache/wiki/plugin/ImageStyleTest.java
new file mode 100644
index 0000000000..9ba73c915f
--- /dev/null
+++ b/jspwiki-main/src/test/java/org/apache/wiki/plugin/ImageStyleTest.java
@@ -0,0 +1,79 @@
+/*
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements. See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership. The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ "License"); you may not use this file except in compliance
+ with the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied. See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+package org.apache.wiki.plugin;
+
+import org.apache.wiki.TestEngine;
+import org.apache.wiki.render.RenderingManager;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+
+/**
+ * Regression tests for the Image plugin style/class/align validation: author CSS must not be able to reposition
+ * content outside the plugin's own box (overlay phishing).
+ */
+public class ImageStyleTest {
+
+ static TestEngine testEngine = TestEngine.build();
+
+ @Test
+ public void overlayStyleIsDropped() throws Exception {
+ final String src = "[{Image src='img.png' link='https://evil.example/fake-login' style='position:fixed;top:0;left:0;width:100vw;height:100vh;z-index:9999;background:#fff'}]";
+ testEngine.saveText( "ImageStylePage1", src );
+ final String res = testEngine.getManager( RenderingManager.class ).getHTML( "ImageStylePage1" );
+ Assertions.assertFalse( res.contains( "position" ), res );
+ Assertions.assertFalse( res.contains( "z-index" ), res );
+ }
+
+ @Test
+ public void benignStyleIsKept() throws Exception {
+ final String src = "[{Image src='img.png' style='width:120px; border: 1px solid'}]";
+ testEngine.saveText( "ImageStylePage2", src );
+ final String res = testEngine.getManager( RenderingManager.class ).getHTML( "ImageStylePage2" );
+ Assertions.assertTrue( res.contains( "width:120px" ), res );
+ }
+
+ @Test
+ public void invalidAlignAndClassAreDropped() throws Exception {
+ final String src = "[{Image src='img.png' align='none;position:fixed' class='x onmouseover=alert(1)'}]";
+ testEngine.saveText( "ImageStylePage3", src );
+ final String res = testEngine.getManager( RenderingManager.class ).getHTML( "ImageStylePage3" );
+ Assertions.assertFalse( res.contains( "position:fixed" ), res );
+ Assertions.assertFalse( res.contains( "onmouseover" ), res );
+ }
+
+ @Test
+ public void isSafeStyleRejectsRepositioningAndEscapes() {
+ Assertions.assertTrue( Image.isSafeStyle( "width:120px; border: 1px solid" ) );
+ Assertions.assertFalse( Image.isSafeStyle( "position:fixed;top:0" ) );
+ Assertions.assertFalse( Image.isSafeStyle( "\\70 osition:fixed" ) );
+ Assertions.assertFalse( Image.isSafeStyle( "margin-top:-9999px" ) );
+ Assertions.assertFalse( Image.isSafeStyle( "background:url(//evil.example/x)" ) );
+ Assertions.assertFalse( Image.isSafeStyle( "transform:translate(-100px,-100px)" ) );
+ }
+
+ @Test
+ public void isSafeCssClassAcceptsIdentifiersOnly() {
+ Assertions.assertTrue( Image.isSafeCssClass( "imageplugin" ) );
+ Assertions.assertTrue( Image.isSafeCssClass( "one two-three" ) );
+ Assertions.assertFalse( Image.isSafeCssClass( "x{color:red}" ) );
+ Assertions.assertFalse( Image.isSafeCssClass( "a;b" ) );
+ }
+
+}