Merge branch 'SECURITY-140'

(JENKINS-33289) NPE when clicking showDiffs SECURITY-140 XSS vulnerability
jenkinsci · Mar 18, 2016 · c363857 · c363857
2 parents 801a6ee + 46a3f34
commit c363857
Show file tree

Hide file tree

Showing 9 changed files with 45 additions and 20 deletions.
diff --git a/pom.xml b/pom.xml
@@ -9,7 +9,8 @@
 
     <artifactId>jobConfigHistory</artifactId>
     <packaging>hpi</packaging>
-    <version>2.13-SNAPSHOT</version>
+    <version>2.14-SNAPSHOT</version>
+
     <name>Jenkins Job Configuration History Plugin</name>
     <description>Saves copies of job and system configurations.</description>
     <url>http://wiki.jenkins-ci.org/display/JENKINS/JobConfigHistory+Plugin</url>
@@ -59,6 +60,7 @@
         </developer>
     </developers>
 
+
     <!-- get every artifact through repo.jenkins-ci.org, which proxies all the artifacts that we need -->
     <repositories>
       <repository>

diff --git a/src/main/java/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction.java b/src/main/java/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction.java
@@ -33,6 +33,8 @@
 import hudson.security.AccessControlled;
 
 import java.io.IOException;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -43,7 +45,6 @@
 
 import jenkins.model.Jenkins;
 
-
 import org.kohsuke.stapler.StaplerRequest;
 import org.kohsuke.stapler.StaplerResponse;
 import org.kohsuke.stapler.export.ExportedBean;
@@ -182,7 +183,18 @@ public final List<ConfigInfo> getSlaveConfigsREST() throws IOException {
      */
     public final String getTimestamp(int timestampNumber) {
         checkConfigurePermission();
-        return this.getRequestParameter("timestamp" + timestampNumber);
+        String timeStamp = this.getRequestParameter("timestamp" + timestampNumber);
+        SimpleDateFormat format = new java.text.SimpleDateFormat("yyyy-MM-dd_HH-mm-ss");
+
+        try{
+            format.setLenient(false);
+            format.parse(timeStamp);
+            return timeStamp;
+        }
+        catch(ParseException e) {
+            return null;
+        }
+
     }
 
     /**

diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryBaseAction.java b/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryBaseAction.java
@@ -202,13 +202,12 @@ protected final String getDiffAsString(final File file1, final File file2,
      */
     public void doDiffFiles(StaplerRequest req, StaplerResponse rsp)
         throws ServletException, IOException {
-        final MultipartFormDataParser parser = new MultipartFormDataParser(req);
-        String timestamp1 = parser.get("timestamp1");
-        String timestamp2 = parser.get("timestamp2");
+        String timestamp1 = req.getParameter("timestamp1");
+        String timestamp2 = req.getParameter("timestamp2");
 
         if (PluginUtils.parsedDate(timestamp1).after(PluginUtils.parsedDate(timestamp2))) {
-            timestamp1 = parser.get("timestamp2");
-            timestamp2 = parser.get("timestamp1");
+            timestamp1 = req.getParameter("timestamp2");
+            timestamp2 = req.getParameter("timestamp1");
         }
         rsp.sendRedirect("showDiffFiles?timestamp1=" + timestamp1
                 + "&timestamp2=" + timestamp2);

diff --git a/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction.java b/src/main/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction.java
@@ -36,6 +36,8 @@
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
 import java.io.InputStream;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -229,10 +231,21 @@ public final String getTimestamp(int timestampNumber) {
             checkConfigurePermission();
             return null;
         }
-        return this.getRequestParameter("timestamp" + timestampNumber);
-    }
+        String timeStamp = this.getRequestParameter("timestamp" + timestampNumber);
+        SimpleDateFormat format = new java.text.SimpleDateFormat("yyyy-MM-dd_HH-mm-ss");
 
-    /**
+        try{
+            format.setLenient(false);
+            format.parse(timeStamp);
+            return timeStamp;
+        }
+        catch(ParseException e) {
+            return null;
+        }
+
+    }
+
+   /**
      * Used in the Difference jelly only. Returns the user that made the change
      * in one of the Files shown in the Difference view(A or B). timestampNumber
      * decides between File A and File B.

diff --git a/src/main/resources/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction/index.jelly b/src/main/resources/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction/index.jelly
@@ -13,7 +13,7 @@
           <j:otherwise>
             <br/>
             <div>
-              <f:form method="post" action="diffFiles" name="diffFiles" enctype="multipart/form-data">
+              <f:form method="post" action="diffFiles" name="diffFiles" >
 	            <j:if test="${configs.size() > 1}">
                   <div align="right">
                     <f:submit value="${%Show Diffs}" />

diff --git a/...resources/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction/showDiffFiles.jelly b/...resources/hudson/plugins/jobConfigHistory/ComputerConfigHistoryAction/showDiffFiles.jelly
@@ -26,8 +26,8 @@
                     <td colspan="2"> <font size="3"> Newer Change </font></td>
                   </tr>
                   <tr>
-                    <td colspan="2"><b>Date:</b> ${timestamp1}</td>
-                    <td colspan="2"><b>Date:</b> ${timestamp2}</td>
+                    <td colspan="2"><b>Date:</b> ${it.getTimestamp(1)}</td>
+                    <td colspan="2"><b>Date:</b> ${it.getTimestamp(2)}</td>
                   </tr>
                   <tr>
                     <td colspan="2"><b>Operation:</b> ${it.getOperation(1)}</td>

diff --git a/src/main/resources/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction/index.jelly b/src/main/resources/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction/index.jelly
@@ -13,7 +13,7 @@
           <j:otherwise>
             <br/>
             <div>
-              <f:form method="post" action="diffFiles" name="diffFiles" enctype="multipart/form-data">
+              <f:form method="post" action="diffFiles" name="diffFiles">
 	            <j:if test="${configs.size() > 1}">
                   <div align="right">
                     <f:submit value="${%Show Diffs}" />

diff --git a/...sources/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction/showDiffFiles.jelly b/...sources/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectAction/showDiffFiles.jelly
@@ -26,8 +26,8 @@
                     <td colspan="2"> <font size="3"> Newer Change </font></td>
                   </tr>
                   <tr>
-                    <td colspan="2"><b>Date:</b> ${timestamp1}</td>
-                    <td colspan="2"><b>Date:</b> ${timestamp2}</td>
+                    <td colspan="2"><b>Date:</b> ${it.getTimestamp(1)}</td>
+                    <td colspan="2"><b>Date:</b> ${it.getTimestamp(2)}</td>
                   </tr>
                   <tr>
                     <td colspan="2"><b>Operation:</b> ${it.getOperation(1)}</td>

diff --git a/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectActionTest.java b/src/test/java/hudson/plugins/jobConfigHistory/JobConfigHistoryProjectActionTest.java
@@ -239,9 +239,8 @@ public void testHasNoConfigurePermission() {
      */
     @Test
     public void testDoDiffFiles() throws Exception {
-        final String boundary = "AAAA";
-        when(mockedRequest.getContentType()).thenReturn("multipart/form-data; boundary=" + boundary);
-        when(mockedRequest.getInputStream()).thenReturn(TUtils.createServletInputStreamFromMultiPartFormData(boundary));
+        when(mockedRequest.getParameter("timestamp1")).thenReturn("2014-02-05_10-42-37");
+        when(mockedRequest.getParameter("timestamp2")).thenReturn("2014-03-12_11-02-12");
         JobConfigHistoryProjectAction sut = createAction();
         sut.doDiffFiles(mockedRequest, mockedResponse);
         verify(mockedResponse).sendRedirect("showDiffFiles?timestamp1=2014-02-05_10-42-37&timestamp2=2014-03-12_11-02-12");