Merge pull request #3312 from jglick/inner-madness-JENKINS-49573

[JENKINS-49795] Fix bad serialization of ParametersAction.parameterDefinitionNames and make sure this kind of mistake produces a warning in the future
jenkinsci · Mar 3, 2018 · 29f146c · 29f146c
2 parents 852ecdc + fcae9bb
commit 29f146c
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 13 deletions.
diff --git a/core/src/main/java/hudson/model/ParametersAction.java b/core/src/main/java/hudson/model/ParametersAction.java
@@ -296,7 +296,7 @@ private Object readResolve() {
     public void onAttached(Run<?, ?> r) {
         ParametersDefinitionProperty p = r.getParent().getProperty(ParametersDefinitionProperty.class);
         if (p != null) {
-            this.parameterDefinitionNames = p.getParameterDefinitionNames();
+            this.parameterDefinitionNames = new ArrayList<>(p.getParameterDefinitionNames());
         } else {
             this.parameterDefinitionNames = Collections.emptyList();
         }

diff --git a/core/src/main/java/jenkins/security/ClassFilterImpl.java b/core/src/main/java/jenkins/security/ClassFilterImpl.java
@@ -156,6 +156,13 @@ public boolean isBlacklisted(Class _c) {
             }
             String location = codeSource(c);
             if (location != null) {
+                if (c.isAnonymousClass()) { // e.g., pkg.Outer$1
+                    LOGGER.warning("JENKINS-49573: attempt to serialize anonymous " + c + " in " + location);
+                } else if (c.isLocalClass()) { // e.g., pkg.Outer$1Local
+                    LOGGER.warning("JENKINS-49573: attempt to serialize local " + c + " in " + location);
+                } else if (c.isSynthetic()) { // e.g., pkg.Outer$$Lambda$1/12345678
+                    LOGGER.warning("JENKINS-49573: attempt to serialize synthetic " + c + " in " + location);
+                }
                 if (isLocationWhitelisted(location)) {
                     LOGGER.log(Level.FINE, "permitting {0} due to its location in {1}", new Object[] {name, location});
                     return false;

diff --git a/test/src/test/java/hudson/model/ParametersAction2Test.java b/test/src/test/java/hudson/model/ParametersAction2Test.java
@@ -2,29 +2,31 @@
 
 import hudson.Functions;
 import hudson.Launcher;
-import hudson.model.queue.QueueTaskFuture;
-import hudson.tasks.BatchFile;
+import hudson.XmlFile;
 import hudson.tasks.Builder;
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+import java.util.logging.Level;
+import static org.hamcrest.CoreMatchers.*;
+import static org.junit.Assert.*;
 import org.junit.Rule;
 import org.junit.Test;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
 import org.jvnet.hudson.test.recipes.LocalData;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.List;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 public class ParametersAction2Test {
     @Rule
     public JenkinsRule j = new JenkinsRule();
 
+    @Rule
+    public LoggerRule logs = new LoggerRule().record("", Level.WARNING).capture(100);
+
     @Test
     @Issue("SECURITY-170")
     public void undefinedParameters() throws Exception {
@@ -309,6 +311,16 @@ public void ensureNoListReuse() throws Exception {
         assertEquals(p2.getLastBuild().getAction(ParametersAction.class).getParameter("foo").getValue(), "for p2");
     }
 
+    @Issue("JENKINS-49573")
+    @Test
+    public void noInnerClasses() throws Exception {
+        FreeStyleProject p = j.createFreeStyleProject();
+        p.addProperty(new ParametersDefinitionProperty(new StringParameterDefinition("key", "sensible-default")));
+        FreeStyleBuild b = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new ParametersAction(new StringParameterValue("key", "value"))));
+        assertThat(new XmlFile(Run.XSTREAM, new File(b.getRootDir(), "build.xml")).asString(), not(containsString("sensible-default")));
+        assertEquals(Collections.emptyList(), logs.getMessages());
+    }
+
     public static boolean hasParameterWithName(Iterable<ParameterValue> values, String name) {
         for (ParameterValue v : values) {
             if (v.getName().equals(name)) {