CVE-2014-2065: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Cross-site scripting (XSS) vulnerability in Jenkins before 1.551 and LTS before 1.532.2 allows remote attackers to inject arbitrary web script or HTML via the iconSize cookie.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
Published: 2014-10-17T15:55Z
Repository: https://github.com/jenkinsci/jenkins
Language: Java
Files changed: core/src/main/java/hudson/Functions.java, core/src/main/java/hudson/model/View.java, core/src/main/java/jenkins/model/Jenkins.java, core/src/main/resources/lib/hudson/setIconSize.jelly
Commit date: 2014-02-11 14:17:00 -0500
Commit message: [FIXED SECURITY-77] XSS in iconSize cookie.

Diff: diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java
index a330a042bf..632961ecab 100644
--- a/core/src/main/java/hudson/Functions.java
+++ b/core/src/main/java/hudson/Functions.java
@@ -121,10 +121,12 @@ import java.util.logging.LogManager;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
 import java.util.logging.SimpleFormatter;
 import java.util.regex.Pattern;
 import org.apache.commons.lang.StringUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
 
 /**
  * Utility functions used in views.
  *
  * <p>
@@ -424,10 +426,19 @@ public class Functions {
         Cookie c = getCookie(req, name);
         if(c==null || c.getValue()==null) return defaultValue;
         return c.getValue();
     }
 
+    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");
+    @Restricted(NoExternalUse.class)
+    public static String validateIconSize(String iconSize) throws SecurityException {
+        if (!ICON_SIZE.matcher(iconSize).matches()) {
+            throw new SecurityException("invalid iconSize");
+        }
+        return iconSize;
+    }
+
     /**
      * Gets the suffix to use for YUI JavaScript.
      */
     public static String getYuiSuffix() {
         return DEBUG_YUI ? "debug" : "min";
diff --git a/core/src/main/java/hudson/model/View.java b/core/src/main/java/hudson/model/View.java
index 7bcc7ce5cb..cae3307659 100644
--- a/core/src/main/java/hudson/model/View.java
+++ b/core/src/main/java/hudson/model/View.java
@@ -764,11 +764,11 @@ public abstract class View extends AbstractModelObject implements AccessControll
             unknown = null;
         }
 
         {
             StaplerRequest req = Stapler.getCurrentRequest();
-            iconSize = req != null ? Functions.getCookie(req, "iconSize", "32x32") : "32x32";
+            iconSize = req != null ? Functions.validateIconSize(Functions.getCookie(req, "iconSize", "32x32")) : "32x32";
         }
 
         @Override protected void compute() throws Exception {
             int itemCount = 0;
             for (Item item : items) {
diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java
index 8d0802736d..ee0098ebe4 100755
--- a/core/src/main/java/jenkins/model/Jenkins.java
+++ b/core/src/main/java/jenkins/model/Jenkins.java
@@ -293,11 +293,10 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.logging.Level;
 import static java.util.logging.Level.SEVERE;
 import java.util.logging.LogRecord;
 import java.util.logging.Logger;
-import java.util.regex.Pattern;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
@@ -3415,13 +3414,13 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
     /**
      * Changes the icon size by changing the cookie
      */
     public void doIconSize( StaplerRequest req, StaplerResponse rsp ) throws IOException, ServletException {
         String qs = req.getQueryString();
-        if(qs==null || !ICON_SIZE.matcher(qs).matches())
+        if(qs==null)
             throw new ServletException();
-        Cookie cookie = new Cookie("iconSize", qs);
+        Cookie cookie = new Cookie("iconSize", Functions.validateIconSize(qs));
         cookie.setMaxAge(/* ~4 mo. */9999999); // #762
         rsp.addCookie(cookie);
         String ref = req.getHeader("Referer");
         if(ref==null)   ref=".";
         rsp.sendRedirect2(ref);
@@ -3962,12 +3961,10 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro
      */
     public static boolean AUTOMATIC_SLAVE_LAUNCH = true;
 
     private static final Logger LOGGER = Logger.getLogger(Jenkins.class.getName());
 
-    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");
-
     public static final PermissionGroup PERMISSIONS = Permission.HUDSON_PERMISSIONS;
     public static final Permission ADMINISTER = Permission.HUDSON_ADMINISTER;
     public static final Permission READ = new Permission(PERMISSIONS,"Read",Messages._Hudson_ReadPermission_Description(),Permission.READ,PermissionScope.JENKINS);
     public static final Permission RUN_SCRIPTS = new Permission(PERMISSIONS, "RunScripts", Messages._Hudson_RunScriptsPermission_Description(),ADMINISTER,PermissionScope.JENKINS);
 
diff --git a/core/src/main/resources/lib/hudson/setIconSize.jelly b/core/src/main/resources/lib/hudson/setIconSize.jelly
index 138c220ae0..bf9e7a0c05 100644
--- a/core/src/main/resources/lib/hudson/setIconSize.jelly
+++ b/core/src/main/resources/lib/hudson/setIconSize.jelly
@@ -25,11 +25,11 @@ THE SOFTWARE.
 <!--
   read icon size from a cookie and set it up
 -->
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:x="jelly:xml" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
-  <j:set scope="parent" var="iconSize" value="${h.getCookie(request,'iconSize','32x32')}" />
+  <j:set scope="parent" var="iconSize" value="${h.validateIconSize(h.getCookie(request,'iconSize','32x32'))}" />
   <!--
     balls look smaller than their actual size,
     so we try not to make the secondary icons look bigger than the icon.
     we want the user's eyes to go to balls, not the clock.
   -->

Vulnerable code:             iconSize = req != null ? Functions.getCookie(req, "iconSize", "32x32") : "32x32";
import java.util.regex.Pattern;
        if(qs==null || !ICON_SIZE.matcher(qs).matches())
        Cookie cookie = new Cookie("iconSize", qs);
    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");

  <j:set scope="parent" var="iconSize" value="${h.getCookie(request,'iconSize','32x32')}" />
Fixed code: import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
    private static final Pattern ICON_SIZE = Pattern.compile("\\d+x\\d+");
    @Restricted(NoExternalUse.class)
    public static String validateIconSize(String iconSize) throws SecurityException {
        if (!ICON_SIZE.matcher(iconSize).matches()) {
            throw new SecurityException("invalid iconSize");
        }
        return iconSize;
    }

            iconSize = req != null ? Functions.validateIconSize(Functions.getCookie(req, "iconSize", "32x32")) : "32x32";
        if(qs==null)
        Cookie cookie = new Cookie("iconSize", Functions.validateIconSize(qs));
  <j:set scope="parent" var="iconSize" value="${h.validateIconSize(h.getCookie(request,'iconSize','32x32'))}" />