Skip to content
Permalink
Browse files
Bulletproof against JENKINS-50752 by catching unserializable step arg…
…uments
  • Loading branch information
svanoort committed Apr 11, 2018
1 parent 64187fd commit e9fe93d891cac44d384bd4ade64ea6fc2c4de1f3
@@ -80,7 +80,8 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
<version>2.25</version>
<!-- Temp version until upstream PR released -->
<version>2.27-20180411.200653-14</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -25,7 +25,9 @@
package org.jenkinsci.plugins.workflow.cps.actions;

import com.google.common.collect.Maps;
import com.google.common.io.NullOutputStream;
import hudson.EnvVars;
import jenkins.model.Jenkins;
import org.jenkinsci.plugins.structs.describable.UninstantiatedDescribable;
import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
import org.jenkinsci.plugins.workflow.steps.Step;
@@ -34,6 +36,7 @@

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@@ -42,6 +45,8 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.logging.Logger;
import java.util.regex.Pattern;


@@ -59,7 +64,7 @@
boolean isUnmodifiedBySanitization = true;

public ArgumentsActionImpl(@Nonnull Map<String, Object> stepArguments, @CheckForNull EnvVars env) {
arguments = sanitizeMapAndRecordMutation(stepArguments, env);
arguments = serializationCheck(sanitizeMapAndRecordMutation(stepArguments, env));
}

/** Create a step, sanitizing strings for secured content */
@@ -267,6 +272,35 @@ Object sanitizeObjectAndRecordMutation(@CheckForNull Object o, @CheckForNull Env
}
}

static final OutputStream NULL_STRM = new NullOutputStream();

/** Verify that all the arguments WILL serialize and if not replace with {@link org.jenkinsci.plugins.workflow.actions.ArgumentsAction.NotStoredReason#UNSERIALIZABLE}
* See JENKINS-50752 for details, but the gist is we need to avoid problems before physical persistence to prevent data loss.
* @return Arguments
*/
Map<String, Object> serializationCheck(@Nonnull Map<String, Object> arguments) {
boolean isMutated = false;
HashMap<String, Object> out = Maps.newHashMapWithExpectedSize(arguments.size());
NullOutputStream strm = new NullOutputStream();
for (Map.Entry<String, Object> entry : arguments.entrySet()) {
Object val = entry.getValue();
try {
if (val != null && !(val instanceof String) && !(val instanceof Boolean) && !(val instanceof Number) && !(val instanceof NotStoredReason) && !(val instanceof TimeUnit)) {
// We only need to check serialization for nontrivial types
Jenkins.XSTREAM2.toXMLUTF8(entry.getValue(), NULL_STRM); // Hacky but can't find a better way
}
out.put(entry.getKey(), entry.getValue());
} catch (Exception ex) {
out.put(entry.getKey(), NotStoredReason.UNSERIALIZABLE);
isMutated = true;
}
}
if (isMutated) {
this.isUnmodifiedBySanitization = false;
}
return out;
}

/**
* Goes through {@link #sanitizeObjectAndRecordMutation(Object, EnvVars)} for each value in a map input.
*/
@@ -9,15 +9,20 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import groovy.lang.MissingMethodException;
import hudson.EnvVars;
import hudson.Functions;
import hudson.XmlFile;
import hudson.model.Action;
import hudson.model.Result;
import hudson.remoting.ProxyException;
import hudson.tasks.ArtifactArchiver;
import jenkins.model.Jenkins;
import org.apache.commons.lang.RandomStringUtils;
import org.hamcrest.Matchers;
import org.jenkinsci.plugins.credentialsbinding.impl.BindingStep;
import org.jenkinsci.plugins.workflow.actions.ArgumentsAction;
import org.jenkinsci.plugins.workflow.actions.ErrorAction;
import org.jenkinsci.plugins.workflow.actions.StageAction;
import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
import org.jenkinsci.plugins.workflow.cps.CpsFlowExecution;
@@ -46,6 +51,10 @@
import org.junit.Rule;
import org.junit.Test;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;

import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
@@ -271,6 +280,22 @@ public void testBasicCreateAndMask() throws Exception {
Assert.assertEquals(ArgumentsAction.NotStoredReason.OVERSIZE_VALUE, argumentsActionImpl.getArgumentValueOrReason("text"));
}

@Test
@Issue("JENKINS-50752")
public void testHandleUnserializableArguments() throws Exception {
HashMap<String, Object> unserializable = new HashMap<String, Object>(3);
Object me = new Object() {
Object writeReplace() {
throw new RuntimeException("Can't serialize me nyah nyah!");
}
};
unserializable.put("ex", me);

ArgumentsActionImpl impl = new ArgumentsActionImpl(unserializable);
Assert.assertEquals(ArgumentsAction.NotStoredReason.UNSERIALIZABLE, impl.getArgumentValueOrReason("ex"));
Assert.assertFalse("Should show argument removed by sanitization", impl.isUnmodifiedArguments());
}

@Test
public void testBasicCredentials() throws Exception {
String username = "bob";

0 comments on commit e9fe93d

Please sign in to comment.