[opencms-dev] DOS attacks made possible by image scaling?

Mika Salminen mika.j.salminen at gmail.com
Thu Jun 11 13:45:24 CEST 2009


Hi,

I created a POC implementation of specifying the allowed scaling paramaters
based on Claus' suggestion.

Restrictions are enabled by putting
<param name="image.scaling.restricted">true</param>
to opencms-vfs.xml

If restrictions are disabled the system should work as before.

If enabled, when CmsImageLoader loads the resource and user has requested it
to be scaled, it first checks if the requested scaling parameter sequence is
allowed. Allowing some parameter sequence is made in resource's or its
parent's properties (you can make the allowance for a whole folder at once).
The parameter sequence can also be given some name.

Resource property that needs to be defined to allow some scaling is named:
image.scaling.restriction. It's format is like following:

w:600,h:600;thumbnail=w:50,h:50

This property value defines that scale parameter "w:600,h:600" is allowed
and also that scale parameter "thumbnail" is allowed and it actually returns
scaled image with parameters "w:50,h:50". So the individual rules are
separated by ";" and parameter sequence names and their values with "=".

Diff agains 7.0.5 is included so you can patch and try it. Probably there
are bugs and missing things but I think that at least the implementation is
quite simple. I haven't tested it a lot, just tried it with a few test cases
manually.

Few things to know about the implementation:
-I haven't put the new member parameter m_restricted  of CmsImageLoader to
every place it possibly belogns.
-Order of scale parameters is significant (w:600,h:600 is different than
h:600,w:600)
-I haven't done anything on JSP side for example to allow cms:img to use
named parameter sequences

Comments/Problems?

-Mika

2009/6/10 Claus Priisholm <cpr at codedroids.com>

> It would probably be easier to do it this way, i.e. "manually" tell
> OpenCms what parameters are acceptable rather than somehow detect
> whether a certain parameter is actually used somewhere in a JSP or it is
> just something a hacker has requested.
>
> For backward compatibility one could just treat the parameter string as
> the key, then one would of course have to define the actual "class"
> parameters under a name like the original parameter string rather than
> something like "thumbnail-profile" (over time that would of course be a
> better practice to use meaningful names but hey, who I am to say that
> "w:800,h:600,t:0,c:c0c0c0" is not meaningful :-)
>
> And if it was controllable through the configuration whether or not to
> use the "indirection" of scaling parameters then all options are open
> for existing sites that are willing to do a bit of gambling.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://webmail.opencms.org/pipermail/opencms-dev/attachments/20090611/ce0bf982/attachment.htm>
-------------- next part --------------
Index: src/org/opencms/loader/CmsImageLoader.java
===================================================================
RCS file: /usr/local/cvs/opencms/src/org/opencms/loader/CmsImageLoader.java,v
retrieving revision 1.10
diff -u -r1.10 CmsImageLoader.java
--- src/org/opencms/loader/CmsImageLoader.java	17 Apr 2008 13:49:14 -0000	1.10
+++ src/org/opencms/loader/CmsImageLoader.java	11 Jun 2009 11:14:04 -0000
@@ -34,6 +34,7 @@
 import org.opencms.cache.CmsVfsNameBasedDiskCache;
 import org.opencms.file.CmsFile;
 import org.opencms.file.CmsObject;
+import org.opencms.file.CmsProperty;
 import org.opencms.file.CmsResource;
 import org.opencms.main.CmsEvent;
 import org.opencms.main.CmsException;
@@ -44,6 +45,7 @@
 import org.opencms.util.CmsStringUtil;
 
 import java.io.IOException;
+import java.util.HashMap;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -84,6 +86,20 @@
 
     /** The configuration parameter for the OpenCms XML configuration to enable the image scaling. */
     public static final String CONFIGURATION_SCALING_ENABLED = "image.scaling.enabled";
+    
+    /** The configuration parameter that restricts the scaling parameters only to the 
+     * parameter sequences defined in resource's or its parent's 
+     * ALLOWED_SCALING_PARAMETERS_RESOURCE_PROPERTY property. */
+    public static final String CONFIGURATION_SCALING_RESTRICTED = "image.scaling.restricted";
+    
+    /** The name of the allowed scaling parameters property of a resource */
+    public static final String ALLOWED_SCALING_PARAMETERS_RESOURCE_PROPERTY = "image.scaling.restriction";
+    
+    /** The restrictions property list delimiter */
+    public static final String ALLOWED_SCALING_PARAMETERS_ELEMENT_DELIMITER = ";";
+    
+    /** The restrictions property list key-value delimiter */
+    public static final String ALLOWED_SCALING_PARAMETERS_ELEMENT_KEY_VALUE_DELIMITER = "=";
 
     /** Default name for the image cache repository. */
     public static final String IMAGE_REPOSITORY_DEFAULT = "/WEB-INF/imagecache/";
@@ -102,6 +118,9 @@
 
     /** Indicates if image scaling is active. */
     protected static boolean m_enabled;
+    
+    /** Indicates that scaling restrictions are enabled. */
+    protected static boolean m_restricted;
 
     /** The maximum image size (width * height) to apply image blurring when down scaling (setting this to high may cause "out of memory" errors). */
     protected static int m_maxBlurSize = CmsImageScaler.SCALE_DEFAULT_MAX_BLUR_SIZE;
@@ -189,6 +208,9 @@
             if (CONFIGURATION_SCALING_ENABLED.equals(paramName)) {
                 m_enabled = Boolean.valueOf(paramValue).booleanValue();
             }
+            if(CONFIGURATION_SCALING_RESTRICTED.equals(paramName)) {
+            	m_restricted = Boolean.valueOf(paramValue).booleanValue();
+            }
             if (CONFIGURATION_IMAGE_FOLDER.equals(paramName)) {
                 m_imageRepositoryFolder = paramValue.trim();
             }
@@ -260,6 +282,7 @@
             result.putAll(config);
         }
         result.put(CONFIGURATION_SCALING_ENABLED, String.valueOf(m_enabled));
+        result.put(CONFIGURATION_SCALING_RESTRICTED, String.valueOf(m_restricted));
         result.put(CONFIGURATION_IMAGE_FOLDER, m_imageRepositoryFolder);
         return result;
     }
@@ -303,20 +326,29 @@
      */
     public void load(CmsObject cms, CmsResource resource, HttpServletRequest req, HttpServletResponse res)
     throws IOException, CmsException {
-
-        if (m_enabled) {
+    	String checkedScaleParameter;
+    	
+    	// Check whether scaling is restricted to only some set of scaling parameter sequences
+    	// or if the scaling parameter is a name of some scale parameter seuqence.
+    	if(m_restricted) {
+    		checkedScaleParameter = checkAllowedScaleParameters(cms, resource, req);
+    	} else {
+    		checkedScaleParameter = req.getParameter(CmsImageScaler.PARAM_SCALE);
+    	}
+    	
+		if(m_enabled && (checkedScaleParameter != null)) {
             if (canSendLastModifiedHeader(resource, req, res)) {
                 // no image processing required at all
                 return;
             }
             // get the scale information from the request
-            CmsImageScaler scaler = new CmsImageScaler(req, m_maxScaleSize, m_maxBlurSize);
+            CmsImageScaler scaler = new CmsImageScaler(checkedScaleParameter, m_maxScaleSize, m_maxBlurSize);
             // load the file from the cache
             CmsFile file = getScaledImage(cms, resource, scaler);
             // now perform standard load operation inherited from dump loader
             super.load(cms, file, req, res);
         } else {
-            // scaling is disabled
+            // scaling is disabled or not available for the resource with given parameters
             super.load(cms, resource, req, res);
         }
     }
@@ -370,4 +402,86 @@
         }
         return file;
     }
+    
+    
+    /**
+     * Checks that the scale parameter sequence in the request is allowed and 
+     * returns the parameter itself if the requested scaling is allowed.
+     * The scale parameter can also be a name of a named scale parameter sequence.
+     * In that case the returned value is the scale parameter sequence mapped to 
+     * the given sequence name.
+     * 
+     * <p>
+     * Allowed scaling parameters are read from resources's 
+     * ALLOWED_SCALING_PARAMETERS_RESOURCE_PROPERTY property. 
+     * Property is also searched from the parents of the resource.
+     * </p>
+     *   
+     * @param cms the initialized CmsObject
+     * @param resource the image resource
+     * @param req servlet request
+     * @return returns null if the scale parameter sequence is not allowed, otherwise returns the checked (and possibly transformed) 
+     * parameter sequence.
+     * 
+     * @throws CmsException in case of errors while accessing OpenCms functions
+     */
+    protected String checkAllowedScaleParameters(CmsObject cms, CmsResource resource, HttpServletRequest req) throws CmsException {
+    	String scaleParameter = req.getParameter(CmsImageScaler.PARAM_SCALE);
+    	// Check that scaling is enabled for the specified resource with given options.
+		CmsProperty restrictionsProperty = cms.readPropertyObject(resource, CmsImageLoader.ALLOWED_SCALING_PARAMETERS_RESOURCE_PROPERTY ,true);
+		
+		if(restrictionsProperty.getValue() == null) {
+			return null;
+		}
+		else {
+			return (String) parseAllowedScaleParameters(restrictionsProperty.getValue()).get(scaleParameter);
+		}
+    }
+    
+    /**
+     * Parses the given allowed scale parameters list retrieved from a resource property and
+     * returns a map containing the parsing result. If map contains key for a given parameter sequence 
+     * or parameter sequence name, the scaling can be allowed.
+     * 
+     * <p>Implementation supports named parameter sequences:<p>
+     * 
+     * <p>For example one can define a resource property with contents</p>
+     * <p><i>thumbnail=w:50,h:50;w:600,h:600</i><p>
+     * <p>This allows querying the thumbnail with by pointing to 
+     * <i>image.jpeg?__scale=thumbnail</i>. It also allows querying image by pointing to
+     * <i>image.jpeg?__scale=w:600,h:600</i> but this parameter sequence has no name.</p> 
+     * 
+     * <p>Map key of the returned map can be some specific name of a parameter sequence or
+     * if the sequence is not given a name the key is the sequence itself as a string.<p>
+     *   
+     * @param allowed scale parameters
+     * @return map of allowed parameter combination names and their values
+     */
+    protected Map parseAllowedScaleParameters(String allowedScaleParametersProperty) {
+    	Map allowedScaleParameters = new HashMap();
+    	
+    	String[] splittedScaleParameters = allowedScaleParametersProperty.split(CmsImageLoader.ALLOWED_SCALING_PARAMETERS_ELEMENT_DELIMITER);	
+    	
+    	for(int i = 0; i < splittedScaleParameters.length; i++) {
+    		
+    		String[] keyAndValue = splittedScaleParameters[i].split(ALLOWED_SCALING_PARAMETERS_ELEMENT_KEY_VALUE_DELIMITER);
+    		String key = null;
+    		String value = null;
+    		// If we have only one part in element, use it also as the key of map element
+    		if(keyAndValue.length == 1) {
+    			key = keyAndValue[0].trim();
+    			value = key;
+    		}
+    		else if(keyAndValue.length > 1) {
+    			key = keyAndValue[0].trim();
+    			value = keyAndValue[1].trim();
+    		}
+    		
+    		// Just a simple check that we do not have null or zero-length key or value
+    		if(key != null && value != null && !"".equals(key) && !"".equals(value)) {
+    			allowedScaleParameters.put(key, value);
+    		}
+    	}
+    	return allowedScaleParameters;
+    }
 }
\ No newline at end of file
Index: src/org/opencms/loader/CmsImageScaler.java
===================================================================
RCS file: /usr/local/cvs/opencms/src/org/opencms/loader/CmsImageScaler.java,v
retrieving revision 1.9
diff -u -r1.9 CmsImageScaler.java
--- src/org/opencms/loader/CmsImageScaler.java	17 Jun 2008 09:59:03 -0000	1.9
+++ src/org/opencms/loader/CmsImageScaler.java	11 Jun 2009 11:14:04 -0000
@@ -232,17 +232,16 @@
     }
 
     /**
-     * Creates a new image scaler based on the given http request.<p>
+     * Creates a new image scaler based on the parameters.<p>
      * 
-     * @param request the http request to read the parameters from
+     * @param parameters the scale parameters
      * @param maxScaleSize the maximum scale size (width or height) for the image
      * @param maxBlurSize the maximum size of the image (width * height) to apply blur (may cause "out of memory" for large images)
      */
-    public CmsImageScaler(HttpServletRequest request, int maxScaleSize, int maxBlurSize) {
+    public CmsImageScaler(String parameters, int maxScaleSize, int maxBlurSize) {
 
         init();
         m_maxBlurSize = maxBlurSize;
-        String parameters = request.getParameter(CmsImageScaler.PARAM_SCALE);
         if (CmsStringUtil.isNotEmpty(parameters)) {
             parseParameters(parameters);
             if (isValid()) {
@@ -254,6 +253,17 @@
             }
         }
     }
+    
+    /**
+     * Creates a new image scaler based on the given http request.<p>
+     * 
+     * @param request the http request to read the parameters from
+     * @param maxScaleSize the maximum scale size (width or height) for the image
+     * @param maxBlurSize the maximum size of the image (width * height) to apply blur (may cause "out of memory" for large images)
+     */
+    public CmsImageScaler(HttpServletRequest request, int maxScaleSize, int maxBlurSize) {
+    	this(request.getParameter(CmsImageScaler.PARAM_SCALE), maxScaleSize, maxBlurSize);
+    }
 
     /**
      * Creates a new image scaler based on the given parameter String.<p>


More information about the opencms-dev mailing list