diff --git a/core/pom.xml b/core/pom.xml
index 258e26f64f..500685db41 100644
--- a/core/pom.xml
+++ b/core/pom.xml
@@ -39,7 +39,7 @@ THE SOFTWARE.
true
- 1.254.1
+ 1.254.2
2.5.6.SEC03
2.4.12
diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
index 00d453f794..ea5177037e 100644
--- a/core/src/main/java/hudson/FilePath.java
+++ b/core/src/main/java/hudson/FilePath.java
@@ -3280,5 +3280,60 @@ public final class FilePath implements Serializable {
return IOUtils.mkdirs(dir);
}
+ /**
+ * Check if the relative child is really a descendant after symlink resolution if any.
+ *
+ * TODO un-restrict it in a weekly after the patch
+ */
+ @Restricted(NoExternalUse.class)
+ public boolean isDescendant(@Nonnull String potentialChildRelativePath) throws IOException, InterruptedException {
+ return act(new IsDescendant(potentialChildRelativePath));
+ }
+
+ private class IsDescendant extends SecureFileCallable {
+ private static final long serialVersionUID = 1L;
+ private String potentialChildRelativePath;
+
+ private IsDescendant(@Nonnull String potentialChildRelativePath){
+ this.potentialChildRelativePath = potentialChildRelativePath;
+ }
+
+ @Override
+ public Boolean invoke(@Nonnull File parentFile, @Nonnull VirtualChannel channel) throws IOException, InterruptedException {
+ if (new File(potentialChildRelativePath).isAbsolute()) {
+ throw new IllegalArgumentException("Only a relative path is supported, the given path is absolute: " + potentialChildRelativePath);
+ }
+
+ Path parent = parentFile.getAbsoluteFile().toPath().normalize();
+
+ String remainingPath = potentialChildRelativePath;
+ File currentFile = parentFile;
+ while (!remainingPath.isEmpty()) {
+ File directChild = this.getDirectChild(currentFile, remainingPath);
+ File childUsingFullPath = new File(currentFile, remainingPath);
+ remainingPath = childUsingFullPath.getAbsolutePath().substring(directChild.getAbsolutePath().length());
+
+ File childFileSymbolic = Util.resolveSymlinkToFile(directChild);
+ if (childFileSymbolic == null) {
+ currentFile = directChild;
+ } else {
+ currentFile = childFileSymbolic;
+ }
+ }
+
+ //TODO could be refactored using Util#isDescendant(File, File) from 2.80+
+ Path child = currentFile.getAbsoluteFile().toPath().normalize();
+ return child.startsWith(parent);
+ }
+
+ private @CheckForNull File getDirectChild(File parentFile, String childPath){
+ File current = new File(parentFile, childPath);
+ while (current != null && !parentFile.equals(current.getParentFile())) {
+ current = current.getParentFile();
+ }
+ return current;
+ }
+ }
+
private static final SoloFilePathFilter UNRESTRICTED = SoloFilePathFilter.wrap(FilePathFilter.UNRESTRICTED);
}
diff --git a/core/src/main/java/hudson/model/AbstractItem.java b/core/src/main/java/hudson/model/AbstractItem.java
index aced1805e5..92303466d0 100644
--- a/core/src/main/java/hudson/model/AbstractItem.java
+++ b/core/src/main/java/hudson/model/AbstractItem.java
@@ -58,6 +58,7 @@ import jenkins.util.xml.XMLUtils;
import org.apache.tools.ant.taskdefs.Copy;
import org.apache.tools.ant.types.FileSet;
+import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.WebMethod;
import org.kohsuke.stapler.export.Exported;
@@ -97,6 +98,8 @@ import javax.xml.transform.stream.StreamSource;
import static hudson.model.queue.Executables.getParentOf;
import hudson.model.queue.SubTask;
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
+import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
+
import org.apache.commons.io.FileUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
@@ -938,6 +941,14 @@ public abstract class AbstractItem extends Actionable implements Item, HttpDelet
@Restricted(NoExternalUse.class)
public Object getTarget() {
if (!SKIP_PERMISSION_CHECK) {
+ if (!getACL().hasPermission(Item.DISCOVER)) {
+ // work around Stapler bug when returning null from getTarget()
+ try {
+ Stapler.getCurrentResponse().sendError(SC_NOT_FOUND); // send same response body Stapler would send
+ } catch (IOException ex) {
+ throw HttpResponses.notFound();
+ }
+ }
getACL().checkPermission(Item.READ);
}
return this;
diff --git a/core/src/main/java/hudson/model/Api.java b/core/src/main/java/hudson/model/Api.java
index b3ce84dfff..024d66869d 100644
--- a/core/src/main/java/hudson/model/Api.java
+++ b/core/src/main/java/hudson/model/Api.java
@@ -135,7 +135,19 @@ public class Api extends AbstractModelObject {
XPath comp = dom.createXPath(xpath);
comp.setFunctionContext(functionContext);
List list = comp.selectNodes(dom);
+
if (wrapper!=null) {
+ // check if the wrapper is a valid entity name
+ // First position: letter or underscore
+ // Other positions: \w (letter, number, underscore), dash or dot
+ String validNameRE = "^[a-zA-Z_][\\w-\\.]*$";
+
+ if(!wrapper.matches(validNameRE)) {
+ rsp.setStatus(HttpServletResponse.SC_BAD_REQUEST);
+ rsp.getWriter().print(Messages.Api_WrapperParamInvalid());
+ return;
+ }
+
Element root = DocumentFactory.getInstance().createElement(wrapper);
for (Object o : list) {
if (o instanceof String) {
diff --git a/core/src/main/java/hudson/model/Descriptor.java b/core/src/main/java/hudson/model/Descriptor.java
index d51edc0c39..124abdc126 100644
--- a/core/src/main/java/hudson/model/Descriptor.java
+++ b/core/src/main/java/hudson/model/Descriptor.java
@@ -39,6 +39,7 @@ import hudson.views.ListViewColumn;
import jenkins.model.GlobalConfiguration;
import jenkins.model.GlobalConfigurationCategory;
import jenkins.model.Jenkins;
+import jenkins.security.RedactSecretJsonInErrorMessageSanitizer;
import jenkins.util.io.OnMaster;
import net.sf.json.JSONArray;
import net.sf.json.JSONObject;
@@ -601,7 +602,7 @@ public abstract class Descriptor> implements Saveable,
} catch (NoSuchMethodException e) {
throw new AssertionError(e); // impossible
} catch (InstantiationException | IllegalAccessException | RuntimeException e) {
- throw new Error("Failed to instantiate "+clazz+" from "+formData,e);
+ throw new Error("Failed to instantiate "+clazz+" from "+RedactSecretJsonInErrorMessageSanitizer.INSTANCE.sanitize(formData),e);
}
}
diff --git a/core/src/main/java/hudson/model/FileParameterValue.java b/core/src/main/java/hudson/model/FileParameterValue.java
index 85bfb04387..f911927285 100644
--- a/core/src/main/java/hudson/model/FileParameterValue.java
+++ b/core/src/main/java/hudson/model/FileParameterValue.java
@@ -36,6 +36,7 @@ import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
+import java.nio.file.Path;
import javax.servlet.ServletException;
import org.apache.commons.fileupload.FileItem;
@@ -45,6 +46,8 @@ import org.apache.commons.fileupload.util.FileItemHeadersImpl;
import org.apache.commons.io.FilenameUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
@@ -61,6 +64,16 @@ import org.kohsuke.stapler.StaplerResponse;
* @author Kohsuke Kawaguchi
*/
public class FileParameterValue extends ParameterValue {
+ private static final String FOLDER_NAME = "fileParameters";
+
+ /**
+ * Escape hatch for SECURITY-1074, fileParameter used to escape their expected folder.
+ * It's not recommended to enable for security reasons. That option is only present for backward compatibility.
+ */
+ @Restricted(NoExternalUse.class)
+ public static /* Script Console modifiable */ boolean ALLOW_FOLDER_TRAVERSAL_OUTSIDE_WORKSPACE =
+ Boolean.getBoolean(FileParameterValue.class.getName() + ".allowFolderTraversalOutsideWorkspace");
+
private transient final FileItem file;
/**
@@ -70,6 +83,9 @@ public class FileParameterValue extends ParameterValue {
/**
* Overrides the location in the build to place this file. Initially set to {@link #getName()}
+ * The location could be directly the filename or also a hierarchical path.
+ * The intermediate folders will be created automatically.
+ * Take care that no escape from the current directory is allowed and will result in the failure of the build.
*/
private String location;
@@ -142,7 +158,16 @@ public class FileParameterValue extends ParameterValue {
public Environment setUp(AbstractBuild build, Launcher launcher, BuildListener listener) throws IOException, InterruptedException {
if (!StringUtils.isEmpty(location) && !StringUtils.isEmpty(file.getName())) {
listener.getLogger().println("Copying file to "+location);
- FilePath locationFilePath = build.getWorkspace().child(location);
+ FilePath ws = build.getWorkspace();
+ if (ws == null) {
+ throw new IllegalStateException("The workspace should be created when setUp method is called");
+ }
+ if (!ALLOW_FOLDER_TRAVERSAL_OUTSIDE_WORKSPACE && !ws.isDescendant(location)) {
+ listener.error("Rejecting file path escaping base directory with relative path: " + location);
+ // force the build to fail
+ return null;
+ }
+ FilePath locationFilePath = ws.child(location);
locationFilePath.getParent().mkdirs();
locationFilePath.copyFrom(file);
locationFilePath.copyTo(new FilePath(getLocationUnderBuild(build)));
@@ -204,6 +229,18 @@ public class FileParameterValue extends ParameterValue {
if (("/" + originalFileName).equals(request.getRestOfPath())) {
AbstractBuild build = (AbstractBuild)request.findAncestor(AbstractBuild.class).getObject();
File fileParameter = getLocationUnderBuild(build);
+
+ if (!ALLOW_FOLDER_TRAVERSAL_OUTSIDE_WORKSPACE) {
+ File fileParameterFolder = getFileParameterFolderUnderBuild(build);
+
+ //TODO can be replaced by Util#isDescendant in 2.80+
+ Path child = fileParameter.getAbsoluteFile().toPath().normalize();
+ Path parent = fileParameterFolder.getAbsoluteFile().toPath().normalize();
+ if (!child.startsWith(parent)) {
+ throw new IllegalStateException("The fileParameter tried to escape the expected folder: " + location);
+ }
+ }
+
if (fileParameter.isFile()) {
try (InputStream data = Files.newInputStream(fileParameter.toPath())) {
long lastModified = fileParameter.lastModified();
@@ -227,7 +264,11 @@ public class FileParameterValue extends ParameterValue {
* @return the location to store the file parameter
*/
private File getLocationUnderBuild(AbstractBuild build) {
- return new File(build.getRootDir(), "fileParameters/" + location);
+ return new File(getFileParameterFolderUnderBuild(build), location);
+ }
+
+ private File getFileParameterFolderUnderBuild(AbstractBuild build){
+ return new File(build.getRootDir(), FOLDER_NAME);
}
/**
diff --git a/core/src/main/java/hudson/model/Run.java b/core/src/main/java/hudson/model/Run.java
index 3fc6905f50..5fdff73308 100644
--- a/core/src/main/java/hudson/model/Run.java
+++ b/core/src/main/java/hudson/model/Run.java
@@ -95,6 +95,8 @@ import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
import static java.util.logging.Level.*;
+import static javax.servlet.http.HttpServletResponse.SC_NOT_FOUND;
+
import java.util.logging.Logger;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
@@ -123,8 +125,10 @@ import org.apache.commons.lang.ArrayUtils;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.stapler.HttpResponse;
+import org.kohsuke.stapler.HttpResponses;
import org.kohsuke.stapler.QueryParameter;
import org.kohsuke.stapler.Stapler;
+import org.kohsuke.stapler.StaplerProxy;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import org.kohsuke.stapler.export.Exported;
@@ -144,7 +148,7 @@ import org.kohsuke.stapler.interceptor.RequirePOST;
*/
@ExportedBean
public abstract class Run ,RunT extends Run>
- extends Actionable implements ExtensionPoint, Comparable, AccessControlled, PersistenceRoot, DescriptorByNameOwner, OnMaster {
+ extends Actionable implements ExtensionPoint, Comparable, AccessControlled, PersistenceRoot, DescriptorByNameOwner, OnMaster, StaplerProxy {
/**
* The original {@link Queue.Item#getId()} has not yet been mapped onto the {@link Run} instance.
@@ -2570,6 +2574,31 @@ public abstract class Run ,RunT extends Run retrieveRedactedKeys(JSONObject jsonObject) {
+ Set redactedKeySet = new HashSet<>();
+ if (jsonObject.has(REDACT_KEY)) {
+ Object value = jsonObject.get(REDACT_KEY);
+ if (value instanceof JSONArray) {
+ for (Object o : jsonObject.getJSONArray(REDACT_KEY)) {
+ if (o instanceof String) {
+ redactedKeySet.add((String) o);
+ } else {
+ // array, object, null, number, boolean
+ LOGGER.log(Level.WARNING, "Unsupported type " + o.getClass().getName() + " for " + REDACT_KEY + ", please use either a single String value or an Array");
+ }
+ }
+ } else if (value instanceof String) {
+ redactedKeySet.add((String) value);
+ } else {
+ // object, null, number, boolean
+ LOGGER.log(Level.WARNING, "Unsupported type " + value.getClass().getName() + " for " + REDACT_KEY + ", please use either a single String value or an Array");
+ }
+ }
+ return redactedKeySet;
+ }
+
+ private Object copyAndSanitize(Object value) {
+ if (value instanceof JSONObject) {
+ return copyAndSanitizeObject((JSONObject) value);
+ } else if (value instanceof JSONArray) {
+ return copyAndSanitizeArray((JSONArray) value);
+ } else {
+ // string, null, number, boolean
+ return value;
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ private JSONObject copyAndSanitizeObject(JSONObject jsonObject) {
+ Set redactedKeySet = retrieveRedactedKeys(jsonObject);
+ JSONObject result = new JSONObject();
+
+ jsonObject.keySet().forEach(keyObject -> {
+ String key = keyObject.toString();
+ if (redactedKeySet.contains(key)) {
+ result.accumulate(key, REDACT_VALUE);
+ } else {
+ Object value = jsonObject.get(keyObject);
+ result.accumulate(key, copyAndSanitize(value));
+ }
+ });
+
+ return result;
+ }
+
+ private JSONArray copyAndSanitizeArray(JSONArray jsonArray) {
+ JSONArray result = new JSONArray();
+
+ jsonArray.forEach(value ->
+ result.add(copyAndSanitize(value))
+ );
+
+ return result;
+ }
+}
diff --git a/core/src/main/resources/hudson/model/Messages.properties b/core/src/main/resources/hudson/model/Messages.properties
index a8f1d9a5aa..191592641f 100644
--- a/core/src/main/resources/hudson/model/Messages.properties
+++ b/core/src/main/resources/hudson/model/Messages.properties
@@ -87,6 +87,7 @@ AbstractProject.LabelLink=Label {1} is serviced by {3,choic
Api.MultipleMatch=XPath "{0}" matched {1} nodes. \
Create XPath that only matches one, or use the "wrapper" query parameter to wrap them all under a root element.
Api.NoXPathMatch=XPath {0} didn\u2019t match
+Api.WrapperParamInvalid=The wrapper parameter can only contain alphanumeric characters or dash/dot/underscore. The first character has to be a letter or underscore.
BallColor.Aborted=Aborted
BallColor.Disabled=Disabled
diff --git a/core/src/main/resources/hudson/model/Messages_es.properties b/core/src/main/resources/hudson/model/Messages_es.properties
index 6d82b5d1aa..b61a753a64 100644
--- a/core/src/main/resources/hudson/model/Messages_es.properties
+++ b/core/src/main/resources/hudson/model/Messages_es.properties
@@ -48,6 +48,7 @@ AbstractProject.ExtendedReadPermission.Description=\
Api.MultipleMatch=XPath "{0}" encontr\u00f3 coincidencias en {1} nodos. \
Crea una expresi\u00f3n XPath que s\u00f3lo encuentre uno, o utiliza el par\u00e1metro "wraper" para que todos los nodos se agrupen bajo un elemento.
Api.NoXPathMatch=XPath {0} no encontr\u00f3 nada
+Api.WrapperParamInvalid=El par\u00E1metro wrapper s\u00F3lo puede contener caracteres alfanum\u00E9ricos o guiones/puntos/subrayado. El primer car\u00E1cter debe ser una letra o subrayado.
BallColor.Aborted=Abortado
BallColor.Disabled=Desactivado
diff --git a/core/src/main/resources/hudson/model/Messages_fr.properties b/core/src/main/resources/hudson/model/Messages_fr.properties
index d1ded2fc0f..7b65394ff0 100644
--- a/core/src/main/resources/hudson/model/Messages_fr.properties
+++ b/core/src/main/resources/hudson/model/Messages_fr.properties
@@ -41,6 +41,7 @@ AbstractProject.WorkspacePermission.Description=\
Api.MultipleMatch=Le XPath "{0}" correspond \u00e0 {1} noeud(s). \
Merci de fournir un XPath qui ne correspond qu''\u00e0 un seul noeud, ou utilisez le param\u00e8tre de requ\u00e8te "wrapper" pour les encapsuler tous dans un \u00e9l\u00e9ment racine.
Api.NoXPathMatch=Pas de correspondance avec le XPath {0}
+Api.WrapperParamInvalid=Le param\u00E8tre wrapper ne doit contenir que des caract\u00E8res alphanum\u00E9riques ainsi que des points / tirets bas / traits d''union, et finalement le premier caract\u00E8re doit \u00EAtre une lettre ou un trait d''union.
BallColor.Aborted=Annul\u00e9
BallColor.Disabled=D\u00e9sactiv\u00e9
@@ -178,3 +179,4 @@ MyViewsProperty.GlobalAction.DisplayName=Mes vues
ManageJenkinsAction.DisplayName=Administrer Jenkins
ParametersDefinitionProperty.DisplayName=Ce build a des paramètres
+
diff --git a/core/src/test/java/jenkins/security/RedactSecretJsonInErrorMessageSanitizerTest.java b/core/src/test/java/jenkins/security/RedactSecretJsonInErrorMessageSanitizerTest.java
new file mode 100644
index 0000000000..9a975481c2
--- /dev/null
+++ b/core/src/test/java/jenkins/security/RedactSecretJsonInErrorMessageSanitizerTest.java
@@ -0,0 +1,139 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2018, CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package jenkins.security;
+
+import net.sf.json.JSONObject;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotSame;
+
+@Issue("SECURITY-765")
+public class RedactSecretJsonInErrorMessageSanitizerTest {
+ @Test
+ public void noSecrets() throws Exception {
+ assertRedaction(
+ "{'a': 1, 'b': '2', 'c': {'c1': 1, 'c2': '2', 'c3': ['3a', '3b']}, 'd': ['4a', {'d1': 1, 'd2': '2'}]}",
+ "{'a': 1, 'b': '2', 'c': {'c1': 1, 'c2': '2', 'c3': ['3a', '3b']}, 'd': ['4a', {'d1': 1, 'd2': '2'}]}"
+ );
+ }
+
+ @Test
+ public void simpleWithSecret() throws Exception {
+ assertRedaction(
+ "{'a': 'secret', 'b': 'other', '$redact': 'a'}",
+ "{'a': '[value redacted]', 'b': 'other', '$redact': 'a'}"
+ );
+ }
+
+ @Test
+ public void singleWithRedactedInArray() throws Exception {
+ assertRedaction(
+ "{'a': 'secret', 'b': 'other', '$redact': ['a']}",
+ "{'a': '[value redacted]', 'b': 'other', '$redact': ['a']}"
+ );
+ }
+
+ @Test
+ public void objectRedactedAcceptedButNotProcessed() throws Exception {
+ assertRedaction(
+ "{'a': 'secret', 'b': 'other', '$redact': {'a': 'a'}}",
+ "{'a': 'secret', 'b': 'other', '$redact': {'a': 'a'}}"
+ );
+ }
+
+ @Test
+ public void weirdValuesInRedactedAcceptedButNotProcessed() throws Exception {
+ assertRedaction(
+ "{'a': 'secret', 'b': 'other', '$redact': [null, true, false, 1, 2, 'a']}",
+ "{'a': '[value redacted]', 'b': 'other', '$redact': [null, true, false, 1, 2, 'a']}"
+ );
+ }
+
+ @Test
+ public void ensureTrueAndOneAsStringAreSupportedAsRedactedKey() throws Exception {
+ //only null is not supported, as passing 'null' is considered as null
+ assertRedaction(
+ "{'true': 'secret1', '1': 'secret3', 'b': 'other', '$redact': ['true', '1']}",
+ "{'true': '[value redacted]', '1': '[value redacted]', 'b': 'other', '$redact': ['true', '1']}"
+ );
+ }
+
+ @Test
+ public void redactFullBranch() throws Exception {
+ assertRedaction(
+ "{'a': {'s1': 'secret1', 's2': 'secret2', 's3': [1,2,3]}, 'b': [4,5,6], 'c': 'other', '$redact': ['a', 'b']}",
+ "{'a': '[value redacted]', 'b': '[value redacted]', 'c': 'other', '$redact': ['a', 'b']}"
+ );
+ }
+
+ @Test
+ public void multipleSecretAtSameLevel() throws Exception {
+ assertRedaction(
+ "{'a1': 'secret1', 'a2': 'secret2', 'b': 'other', '$redact': ['a1', 'a2']}",
+ "{'a1': '[value redacted]', 'a2': '[value redacted]', 'b': 'other', '$redact': ['a1', 'a2']}"
+ );
+ }
+
+ @Test
+ public void redactedKeyWithoutCorrespondences() throws Exception {
+ assertRedaction(
+ "{'a1': 'secret1', 'a2': 'secret2', 'b': 'other', '$redact': ['a0', 'a1', 'a2', 'a3']}",
+ "{'a1': '[value redacted]', 'a2': '[value redacted]', 'b': 'other', '$redact': ['a0', 'a1', 'a2', 'a3']}"
+ );
+ }
+
+ @Test
+ public void secretsAtMultipleLevels() throws Exception {
+ assertRedaction(
+ "{'a1': 'secret1', 'a2': 'secret2', 'b': 'other', '$redact': ['a1', 'a2'], 'sub': {'c1': 'secret1', 'c2': 'secret2', 'c3': 'other', '$redact': ['c1', 'c2']}}",
+ "{'a1': '[value redacted]', 'a2': '[value redacted]', 'b': 'other', '$redact': ['a1', 'a2'], 'sub': {'c1': '[value redacted]', 'c2': '[value redacted]', 'c3': 'other', '$redact': ['c1', 'c2']}}"
+ );
+ }
+
+ @Test
+ public void noInteractionBetweenLevels() throws Exception {
+ assertRedaction(
+ "{'a': 'secret', 'b': 'other', 'c': 'other', '$redact': 'a', 'sub': {'a': 'other', 'b': 'secret', 'c': 'other', '$redact': 'b'}}",
+ "{'a': '[value redacted]', 'b': 'other', 'c': 'other', '$redact': 'a', 'sub': {'a': 'other', 'b': '[value redacted]', 'c': 'other', '$redact': 'b'}}"
+ );
+ }
+
+ @Test
+ public void deeplyNestedObject() throws Exception {
+ assertRedaction(
+ "{'sub': {'arr': ['d1', 2, {'a1': 'other', 'b1':'other', 'c1': 'secret', '$redact': 'c1'}, 4, {'a2': 'other', 'b2': 'other', 'c2': 'secret', '$redact': 'c2'}]}, '$redact': 'b'}",
+ "{'sub': {'arr': ['d1', 2, {'a1': 'other', 'b1':'other', 'c1': '[value redacted]', '$redact': 'c1'}, 4, {'a2': 'other', 'b2': 'other', 'c2': '[value redacted]', '$redact': 'c2'}]}, '$redact': 'b'}"
+ );
+ }
+
+ private void assertRedaction(String from, String to) {
+ JSONObject input = JSONObject.fromObject(from.replace('\'', '"'));
+ JSONObject output = RedactSecretJsonInErrorMessageSanitizer.INSTANCE.sanitize(input);
+ assertNotSame(output, input);
+ assertEquals("redaction of " + from, to.replace('\'', '"').replace(" ", ""),
+ output.toString().replace(" ", ""));
+ }
+}
diff --git a/test/src/test/java/hudson/AbstractItemSecurity1114Test.java b/test/src/test/java/hudson/AbstractItemSecurity1114Test.java
new file mode 100644
index 0000000000..c6d0e2199e
--- /dev/null
+++ b/test/src/test/java/hudson/AbstractItemSecurity1114Test.java
@@ -0,0 +1,97 @@
+package hudson;
+
+import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException;
+import hudson.model.AbstractItem;
+import hudson.model.Item;
+import hudson.model.RootAction;
+import hudson.security.ACL;
+import hudson.security.ACLContext;
+import jenkins.model.Jenkins;
+import org.junit.Assert;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.For;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockAuthorizationStrategy;
+import org.jvnet.hudson.test.TestExtension;
+
+import javax.annotation.CheckForNull;
+
+public class AbstractItemSecurity1114Test {
+ @Rule
+ public JenkinsRule j = new JenkinsRule();
+
+ @Test
+ @Issue("SECURITY-1114")
+ @For(AbstractItem.class)
+ public void testAccess() throws Exception {
+ j.jenkins.setSecurityRealm(j.createDummySecurityRealm());
+ MockAuthorizationStrategy authorizationStrategy = new MockAuthorizationStrategy();
+ authorizationStrategy.grant(Jenkins.READ).onRoot().toEveryone();
+ authorizationStrategy.grant(Item.DISCOVER).everywhere().to("alice");
+ authorizationStrategy.grant(Item.READ).everywhere().to("bob");
+ j.jenkins.setAuthorizationStrategy(authorizationStrategy);
+
+ j.createFreeStyleProject("myproject");
+
+ // alice can discover project
+ JenkinsRule.WebClient wc = j.createWebClient().login("alice");
+ try {
+ wc.goTo("bypass/myproject");
+ Assert.fail("expected exception");
+ } catch (FailingHttpStatusCodeException e) {
+ Assert.assertEquals("alice can discover", 403, e.getStatusCode());
+ }
+
+ // bob can read project
+ wc = j.createWebClient().login("bob");
+ wc.goTo("bypass/myproject"); // success
+
+
+ // carol has no permissions
+ wc = j.createWebClient().login("carol");
+ try {
+ wc.goTo("bypass/nonexisting");
+ Assert.fail("expected exception");
+ } catch (FailingHttpStatusCodeException e) {
+ Assert.assertEquals("carol gets 404 for nonexisting project", 404, e.getStatusCode());
+ }
+ try {
+ wc.goTo("bypass/myproject");
+ Assert.fail("expected exception");
+ } catch (FailingHttpStatusCodeException e) {
+ Assert.assertEquals("carol gets 404 for invisible project", 404, e.getStatusCode());
+ }
+
+ }
+
+ @TestExtension
+ public static class BypassAccess implements RootAction {
+ public Item getDynamic(String name) {
+ Item item;
+ try (ACLContext ctx = ACL.as(ACL.SYSTEM)) {
+ item = Jenkins.getInstance().getItemByFullName(name);
+ }
+ return item;
+ }
+
+ @CheckForNull
+ @Override
+ public String getIconFileName() {
+ return null;
+ }
+
+ @CheckForNull
+ @Override
+ public String getDisplayName() {
+ return null;
+ }
+
+ @CheckForNull
+ @Override
+ public String getUrlName() {
+ return "bypass";
+ }
+ }
+}
diff --git a/test/src/test/java/hudson/PluginTest_SEC925.java b/test/src/test/java/hudson/PluginSEC925Test.java
similarity index 97%
rename from test/src/test/java/hudson/PluginTest_SEC925.java
rename to test/src/test/java/hudson/PluginSEC925Test.java
index f52abce238..6ecf68ae1b 100644
--- a/test/src/test/java/hudson/PluginTest_SEC925.java
+++ b/test/src/test/java/hudson/PluginSEC925Test.java
@@ -13,7 +13,7 @@ import java.util.List;
import java.util.concurrent.Future;
//TODO merge it within PluginTest after the security release
-public class PluginTest_SEC925 {
+public class PluginSEC925Test {
@Rule
public JenkinsRule r = new JenkinsRule();
diff --git a/test/src/test/java/hudson/model/ApiSecurity1129Test.java b/test/src/test/java/hudson/model/ApiSecurity1129Test.java
new file mode 100644
index 0000000000..999eacb43e
--- /dev/null
+++ b/test/src/test/java/hudson/model/ApiSecurity1129Test.java
@@ -0,0 +1,86 @@
+package hudson.model;
+
+import com.gargoylesoftware.htmlunit.WebResponse;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.xml.sax.SAXException;
+
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+//TODO after the security fix, it could be merged inside ApiTest
+public class ApiSecurity1129Test {
+ @Rule
+ public JenkinsRule j = new JenkinsRule();
+
+ /**
+ * Test the wrapper parameter for the api/xml urls to avoid XSS.
+ * @throws Exception See {@link #checkWrapperParam(String, Integer, String)}
+ */
+ @Issue("SECURITY-1129")
+ @Test
+ public void wrapperXss() throws Exception {
+ String wrapper = "html%20xmlns=\"http://www.w3.org/1999/xhtml\">