From 0530a6645aac10fec005614211660e98db44b5eb Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 12:40:18 -0500 Subject: [PATCH 01/15] [FIXED SECURITY-73] Require RUN_SCRIPTS for /heapDump. --- core/src/main/java/hudson/util/RemotingDiagnostics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/util/RemotingDiagnostics.java b/core/src/main/java/hudson/util/RemotingDiagnostics.java index 408c9931f4..8f014bbcf8 100644 --- a/core/src/main/java/hudson/util/RemotingDiagnostics.java +++ b/core/src/main/java/hudson/util/RemotingDiagnostics.java @@ -202,7 +202,7 @@ public final class RemotingDiagnostics { @WebMethod(name="heapdump.hprof") public void doHeapDump(StaplerRequest req, StaplerResponse rsp) throws IOException, InterruptedException { - owner.checkPermission(Jenkins.ADMINISTER); + owner.checkPermission(Jenkins.RUN_SCRIPTS); rsp.setContentType("application/octet-stream"); FilePath dump = obtain(); From 5d57c855f3147bfc5e7fda9252317b428a700014 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 14:33:10 -0500 Subject: [PATCH 02/15] [FIXED SECURITY-74] Apply markup formatter to remote cause note. --- core/src/main/java/hudson/model/Cause.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/core/src/main/java/hudson/model/Cause.java b/core/src/main/java/hudson/model/Cause.java index 4ccab03338..52b2eeffb1 100644 --- a/core/src/main/java/hudson/model/Cause.java +++ b/core/src/main/java/hudson/model/Cause.java @@ -34,6 +34,7 @@ import jenkins.model.Jenkins; import org.kohsuke.stapler.export.Exported; import org.kohsuke.stapler.export.ExportedBean; import com.thoughtworks.xstream.converters.UnmarshallingContext; +import java.io.IOException; import java.util.HashSet; import java.util.Set; import javax.annotation.CheckForNull; @@ -371,10 +372,13 @@ public abstract class Cause { @Override public String getShortDescription() { if(note != null) { - return Messages.Cause_RemoteCause_ShortDescriptionWithNote(addr, note); - } else { - return Messages.Cause_RemoteCause_ShortDescription(addr); + try { + return Messages.Cause_RemoteCause_ShortDescriptionWithNote(addr, Jenkins.getInstance().getMarkupFormatter().translate(note)); + } catch (IOException x) { + // ignore + } } + return Messages.Cause_RemoteCause_ShortDescription(addr); } @Override From 8ac74c350779921598f9d5edfed39dd35de8842a Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Thu, 13 Jun 2013 21:45:57 +0200 Subject: [PATCH 03/15] [FIXED SECURITY-75] Invalidate session after login to avoid session fixation --- .../java/hudson/security/AuthenticationProcessingFilter2.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java index 7f282199e8..1819c771f4 100644 --- a/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java +++ b/core/src/main/java/hudson/security/AuthenticationProcessingFilter2.java @@ -85,6 +85,7 @@ public class AuthenticationProcessingFilter2 extends AuthenticationProcessingFil // HttpSessionContextIntegrationFilter stores the updated SecurityContext object into this session later // (either when a redirect is issued, via its HttpResponseWrapper, or when the execution returns to its // doFilter method. + request.getSession().invalidate(); request.getSession(); } From 535c1115bbf07f8a57d509f2d00598d6e21870d4 Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Sat, 15 Jun 2013 01:15:46 +0200 Subject: [PATCH 04/15] [FIXED SECURITY-76] Prevent iframe injection, forbid iframe by default --- core/src/main/java/hudson/markup/MyspacePolicy.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/hudson/markup/MyspacePolicy.java b/core/src/main/java/hudson/markup/MyspacePolicy.java index f0543f9538..6672d1f89e 100644 --- a/core/src/main/java/hudson/markup/MyspacePolicy.java +++ b/core/src/main/java/hudson/markup/MyspacePolicy.java @@ -66,7 +66,6 @@ public class MyspacePolicy { tag("span,div"); tag("img", "src",ONSITE_OR_OFFSITE_URL, "hspace","vspace"); - tag("iframe", "src"); tag("link", "type","rel"); tag("ul,ol,li,dd,dl,dt,thead,tbody,tfoot"); tag("table", "noresize"); From a0b00508eeb74d7033dc4100eb382df4e8fa72e7 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 15:16:41 -0500 Subject: [PATCH 05/15] [FIXED SECURITY-77] XSS in iconSize cookie. --- core/src/main/java/hudson/Functions.java | 11 +++++++++++ core/src/main/java/hudson/model/View.java | 2 +- core/src/main/java/jenkins/model/Jenkins.java | 7 ++----- core/src/main/resources/lib/hudson/setIconSize.jelly | 2 +- 4 files changed, 15 insertions(+), 7 deletions(-) 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 @@ -123,6 +123,8 @@ 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. @@ -426,6 +428,15 @@ public class Functions { 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. */ 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 @@ -766,7 +766,7 @@ public abstract class View extends AbstractModelObject implements AccessControll { 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 { 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 @@ -295,7 +295,6 @@ 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; @@ -3417,9 +3416,9 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro */ 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"); @@ -3964,8 +3963,6 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro 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); 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 @@ -27,7 +27,7 @@ THE SOFTWARE. --> - + From 788b7d7a067fad4972fefaaa527141847bfeff55 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 15:24:35 -0500 Subject: [PATCH 08/15] [FIXED SECURITY-88] Forbid offsite-bound forms in default markup formatter policy. --- core/src/main/java/hudson/markup/MyspacePolicy.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/markup/MyspacePolicy.java b/core/src/main/java/hudson/markup/MyspacePolicy.java index 6672d1f89e..e2b0bba9e1 100644 --- a/core/src/main/java/hudson/markup/MyspacePolicy.java +++ b/core/src/main/java/hudson/markup/MyspacePolicy.java @@ -48,7 +48,7 @@ public class MyspacePolicy { "script","noscript",/*"iframe",*/"frameset","frame"); tag("label", "for"); - tag("form", "action",ONSITE_OR_OFFSITE_URL, + tag("form", "action",ONSITE_URL, "method"); tag("button", "value", "type"); tag("input", "maxlength","checked", From 5548b5220cfd496831b5721124189ff18fbb12a3 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 12:57:58 -0500 Subject: [PATCH 09/15] [FIXED SECURITY-89] When checking an API token, verify that the user actually exists. --- .../java/jenkins/security/ApiTokenFilter.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/core/src/main/java/jenkins/security/ApiTokenFilter.java b/core/src/main/java/jenkins/security/ApiTokenFilter.java index 630ae5ec04..b1078e6240 100644 --- a/core/src/main/java/jenkins/security/ApiTokenFilter.java +++ b/core/src/main/java/jenkins/security/ApiTokenFilter.java @@ -2,9 +2,13 @@ package jenkins.security; import hudson.model.User; import hudson.security.ACL; +import hudson.security.UserMayOrMayNotExistException; import hudson.util.Scrambler; +import jenkins.model.Jenkins; import org.acegisecurity.context.SecurityContext; import org.acegisecurity.context.SecurityContextHolder; +import org.acegisecurity.userdetails.UsernameNotFoundException; +import org.springframework.dao.DataAccessException; import javax.servlet.Filter; import javax.servlet.FilterChain; @@ -41,6 +45,17 @@ public class ApiTokenFilter implements Filter { int idx = uidpassword.indexOf(':'); if (idx >= 0) { String username = uidpassword.substring(0, idx); + try { + Jenkins.getInstance().getSecurityRealm().loadUserByUsername(username); + } catch (UserMayOrMayNotExistException x) { + // OK, give them the benefit of the doubt. + } catch (UsernameNotFoundException x) { + // Not/no longer a user; deny the API token. (But do not leak the information that this happened.) + chain.doFilter(request, response); + return; + } catch (DataAccessException x) { + throw new ServletException(x); + } String password = uidpassword.substring(idx+1); // attempt to authenticate as API token From bf539198564a1108b7b71a973bf7de963a6213ef Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 7 Feb 2014 15:28:05 -0500 Subject: [PATCH 10/15] [FIXED SECURITY-93] PasswordParameterDefinition should serve existing default value in encrypted form. And strengthen functional tests (using configRoundTrip) to ensure that the same mistake is not made elsewhere. --- core/src/main/java/hudson/Functions.java | 3 ++ .../model/PasswordParameterDefinition.java | 7 +++ .../PasswordParameterDefinition/config.jelly | 2 +- .../PasswordParameterDefinitionTest.java | 43 +++++++++++++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java diff --git a/core/src/main/java/hudson/Functions.java b/core/src/main/java/hudson/Functions.java index 632961ecab..996a70900f 100644 --- a/core/src/main/java/hudson/Functions.java +++ b/core/src/main/java/hudson/Functions.java @@ -1464,6 +1464,9 @@ public class Functions { public String getPasswordValue(Object o) { if (o==null) return null; if (o instanceof Secret) return ((Secret)o).getEncryptedValue(); + if (getIsUnitTest()) { + throw new SecurityException("attempted to render plaintext ‘" + o + "’ in password field; use a getter of type Secret instead"); + } return o.toString(); } diff --git a/core/src/main/java/hudson/model/PasswordParameterDefinition.java b/core/src/main/java/hudson/model/PasswordParameterDefinition.java index 6c3388cd5e..ce93144a56 100644 --- a/core/src/main/java/hudson/model/PasswordParameterDefinition.java +++ b/core/src/main/java/hudson/model/PasswordParameterDefinition.java @@ -28,6 +28,8 @@ import org.kohsuke.stapler.StaplerRequest; import org.kohsuke.stapler.DataBoundConstructor; import hudson.Extension; import hudson.util.Secret; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.DoNotUse; /** * Parameter whose value is a {@link Secret} and is hidden from the UI. @@ -76,6 +78,11 @@ public class PasswordParameterDefinition extends SimpleParameterDefinition { return Secret.toString(defaultValue); } + @Restricted(DoNotUse.class) // used from Jelly + public Secret getDefaultValueAsSecret() { + return defaultValue; + } + // kept for backward compatibility public void setDefaultValue(String defaultValue) { this.defaultValue = Secret.fromString(defaultValue); diff --git a/core/src/main/resources/hudson/model/PasswordParameterDefinition/config.jelly b/core/src/main/resources/hudson/model/PasswordParameterDefinition/config.jelly index 2ccb38245e..53b9fd8557 100644 --- a/core/src/main/resources/hudson/model/PasswordParameterDefinition/config.jelly +++ b/core/src/main/resources/hudson/model/PasswordParameterDefinition/config.jelly @@ -30,7 +30,7 @@ THE SOFTWARE. - + diff --git a/test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java b/test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java new file mode 100644 index 0000000000..8b4dcb2906 --- /dev/null +++ b/test/src/test/java/hudson/model/PasswordParameterDefinitionTest.java @@ -0,0 +1,43 @@ +/* + * The MIT License + * + * Copyright 2013 Jesse Glick. + * + * 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 hudson.model; + +import static org.junit.Assert.assertEquals; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +public class PasswordParameterDefinitionTest { + + @Rule public JenkinsRule j = new JenkinsRule(); + + @Test public void defaultValueKeptSecret() throws Exception { + FreeStyleProject p = j.createFreeStyleProject(); + p.addProperty(new ParametersDefinitionProperty(new PasswordParameterDefinition("p", "s3cr3t", ""))); + j.configRoundtrip(p); + assertEquals("s3cr3t", ((PasswordParameterDefinition) p.getProperty(ParametersDefinitionProperty.class).getParameterDefinition("p")).getDefaultValue()); + } + +} From 29351af4bd01f61715418916fc12c52be46bd9b0 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 11 Feb 2014 14:20:38 -0500 Subject: [PATCH 11/15] [FIXED SECURITY-106] Jenkins on Winstone vulnerable to session hijacking. --- war/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/war/pom.xml b/war/pom.xml index 5aa08c51db..b928e1a1a7 100644 --- a/war/pom.xml +++ b/war/pom.xml @@ -104,7 +104,7 @@ THE SOFTWARE. --> org.jenkins-ci winstone - 0.9.10-jenkins-43 + 0.9.10-jenkins-48 test From 622e39f8b4c4764a0768bf58645767405ba9ccaa Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 10 Feb 2014 22:28:53 -0500 Subject: [PATCH 12/15] [FIXED SECURITY-107] When security-related fields in Jenkins cannot be unmarshaled, it is best to halt startup. --- .../util/RobustReflectionConverter.java | 24 +++++++++++++++++++ core/src/main/java/hudson/util/XStream2.java | 12 ++++++++++ core/src/main/java/jenkins/model/Jenkins.java | 2 ++ 3 files changed, 38 insertions(+) diff --git a/core/src/main/java/hudson/util/RobustReflectionConverter.java b/core/src/main/java/hudson/util/RobustReflectionConverter.java index 5a3a70d5d2..569e533169 100755 --- a/core/src/main/java/hudson/util/RobustReflectionConverter.java +++ b/core/src/main/java/hudson/util/RobustReflectionConverter.java @@ -45,6 +45,7 @@ import hudson.model.Saveable; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -73,6 +74,8 @@ public class RobustReflectionConverter implements Converter { protected transient SerializationMethodInvoker serializationMethodInvoker; private transient ReflectionProvider pureJavaReflectionProvider; private final @Nonnull XStream2.ClassOwnership classOwnership; + /** {@code pkg.Clazz#fieldName} */ + private final Set criticalFields = Collections.synchronizedSet(new HashSet()); public RobustReflectionConverter(Mapper mapper, ReflectionProvider reflectionProvider) { this(mapper, reflectionProvider, new XStream2().new PluginClassOwnership()); @@ -85,6 +88,10 @@ public class RobustReflectionConverter implements Converter { serializationMethodInvoker = new SerializationMethodInvoker(); } + void addCriticalField(Class clazz, String field) { + criticalFields.add(clazz.getName() + '#' + field); + } + public boolean canConvert(Class type) { return true; } @@ -261,8 +268,16 @@ public class RobustReflectionConverter implements Converter { while (reader.hasMoreChildren()) { reader.moveDown(); + boolean critical = false; try { String fieldName = mapper.realMember(result.getClass(), reader.getNodeName()); + for (Class concrete = result.getClass(); concrete != null; concrete = concrete.getSuperclass()) { + // Not quite right since a subclass could shadow a field, but probably suffices: + if (criticalFields.contains(concrete.getName() + '#' + fieldName)) { + critical = true; + break; + } + } boolean implicitCollectionHasSameName = mapper.getImplicitCollectionDefForFieldName(result.getClass(), reader.getNodeName()) != null; Class classDefiningField = determineWhichClassDefinesField(reader); @@ -294,12 +309,21 @@ public class RobustReflectionConverter implements Converter { } } } catch (MissingFieldException e) { + if (critical) { + throw e; + } LOGGER.log(WARNING,"Skipping a non-existent field "+e.getFieldName(),e); addErrorInContext(context, e); } catch (CannotResolveClassException e) { + if (critical) { + throw e; + } LOGGER.log(WARNING,"Skipping a non-existent type",e); addErrorInContext(context, e); } catch (LinkageError e) { + if (critical) { + throw e; + } LOGGER.log(WARNING,"Failed to resolve a type",e); addErrorInContext(context, e); } diff --git a/core/src/main/java/hudson/util/XStream2.java b/core/src/main/java/hudson/util/XStream2.java index c4543979b1..6602e53405 100644 --- a/core/src/main/java/hudson/util/XStream2.java +++ b/core/src/main/java/hudson/util/XStream2.java @@ -65,6 +65,8 @@ import java.nio.charset.Charset; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import javax.annotation.CheckForNull; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; /** * {@link XStream} enhanced for additional Java5 support and improved robustness. @@ -121,6 +123,16 @@ public class XStream2 extends XStream { return reflectionConverter; } + /** + * Specifies that a given field of a given class should not be treated with laxity by {@link RobustCollectionConverter}. + * @param clazz a class which we expect to hold a non-{@code transient} field + * @param field a field name in that class + */ + @Restricted(NoExternalUse.class) // TODO could be opened up later + public void addCriticalField(Class clazz, String field) { + reflectionConverter.addCriticalField(clazz, field); + } + static String trimVersion(String version) { // XXX seems like there should be some trick with VersionNumber to do this return version.replaceFirst(" .+$", ""); diff --git a/core/src/main/java/jenkins/model/Jenkins.java b/core/src/main/java/jenkins/model/Jenkins.java index ee0098ebe4..712cd37eed 100755 --- a/core/src/main/java/jenkins/model/Jenkins.java +++ b/core/src/main/java/jenkins/model/Jenkins.java @@ -3985,6 +3985,8 @@ public class Jenkins extends AbstractCIBase implements ModifiableTopLevelItemGro // for backward compatibility with <1.75, recognize the tag name "view" as well. XSTREAM.alias("view", ListView.class); XSTREAM.alias("listView", ListView.class); + XSTREAM2.addCriticalField(Jenkins.class, "securityRealm"); + XSTREAM2.addCriticalField(Jenkins.class, "authorizationStrategy"); // this seems to be necessary to force registration of converter early enough Mode.class.getEnumConstants(); From ad38d8480f20ce3cbf8fec3e2003bc83efda4f7d Mon Sep 17 00:00:00 2001 From: Vojtech Juranek Date: Fri, 7 Feb 2014 00:46:07 +0100 Subject: [PATCH 13/15] [FIXED SECURITY-108] Check job name created via CLI --- core/src/main/java/hudson/cli/CreateJobCommand.java | 1 + 1 file changed, 1 insertion(+) diff --git a/core/src/main/java/hudson/cli/CreateJobCommand.java b/core/src/main/java/hudson/cli/CreateJobCommand.java index db73c30b51..02dd6ae55e 100644 --- a/core/src/main/java/hudson/cli/CreateJobCommand.java +++ b/core/src/main/java/hudson/cli/CreateJobCommand.java @@ -72,6 +72,7 @@ public class CreateJobCommand extends CLICommand { name = name.substring(i + 1); } + Jenkins.checkGoodName(name); ig.createProjectFromXML(name, stdin); return 0; } From b6b2a367a7976be80a799c6a49fa6c58d778b50e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 10 Feb 2014 19:13:32 -0500 Subject: [PATCH 14/15] [FIXED SECURITY-109] SECURITY-55 fix to BuildTrigger configuration failed if downstream project was not visible. --- .../java/hudson/model/AbstractProject.java | 9 +++- .../java/hudson/tasks/BuildTriggerTest.java | 44 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/hudson/model/AbstractProject.java b/core/src/main/java/hudson/model/AbstractProject.java index 99fb7054fa..bb5e06d998 100644 --- a/core/src/main/java/hudson/model/AbstractProject.java +++ b/core/src/main/java/hudson/model/AbstractProject.java @@ -1939,7 +1939,14 @@ public abstract class AbstractProject

,R extends A for (Publisher _t : Descriptor.newInstancesFromHeteroList(req, json, "publisher", Jenkins.getInstance().getExtensionList(BuildTrigger.DescriptorImpl.class))) { BuildTrigger t = (BuildTrigger) _t; - for (AbstractProject downstream : t.getChildProjects(this)) { + List childProjects; + SecurityContext orig = ACL.impersonate(ACL.SYSTEM); + try { + childProjects = t.getChildProjects(this); + } finally { + SecurityContextHolder.setContext(orig); + } + for (AbstractProject downstream : childProjects) { downstream.checkPermission(BUILD); } } diff --git a/test/src/test/java/hudson/tasks/BuildTriggerTest.java b/test/src/test/java/hudson/tasks/BuildTriggerTest.java index f9df59e122..2d4f5e5fcb 100644 --- a/test/src/test/java/hudson/tasks/BuildTriggerTest.java +++ b/test/src/test/java/hudson/tasks/BuildTriggerTest.java @@ -23,14 +23,26 @@ */ package hudson.tasks; +import com.gargoylesoftware.htmlunit.FailingHttpStatusCodeException; import com.gargoylesoftware.htmlunit.html.HtmlForm; import com.gargoylesoftware.htmlunit.html.HtmlPage; +import com.gargoylesoftware.htmlunit.html.HtmlTextInput; import hudson.maven.MavenModuleSet; import hudson.maven.MavenModuleSetBuild; import hudson.model.FreeStyleBuild; import hudson.model.FreeStyleProject; +import hudson.model.Item; import hudson.model.Result; import hudson.model.Run; +import hudson.security.AuthorizationMatrixProperty; +import hudson.security.LegacySecurityRealm; +import hudson.security.Permission; +import hudson.security.ProjectMatrixAuthorizationStrategy; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import jenkins.model.Jenkins; import org.jvnet.hudson.test.ExtractResourceSCM; import org.jvnet.hudson.test.HudsonTestCase; import org.jvnet.hudson.test.MockBuilder; @@ -131,4 +143,36 @@ public class BuildTriggerTest extends HudsonTestCase { public void testMavenTriggerEvenWhenUnstable() throws Exception { doMavenTriggerTest(true); } + + public void testConfigureDownstreamProjectSecurity() throws Exception { + jenkins.setSecurityRealm(new LegacySecurityRealm()); + ProjectMatrixAuthorizationStrategy auth = new ProjectMatrixAuthorizationStrategy(); + auth.add(Jenkins.READ, "alice"); + jenkins.setAuthorizationStrategy(auth); + FreeStyleProject upstream = createFreeStyleProject("upstream"); + Map> perms = new HashMap>(); + perms.put(Item.READ, Collections.singleton("alice")); + perms.put(Item.CONFIGURE, Collections.singleton("alice")); + upstream.addProperty(new AuthorizationMatrixProperty(perms)); + FreeStyleProject downstream = createFreeStyleProject("downstream"); + /* Original SECURITY-55 test case: + downstream.addProperty(new AuthorizationMatrixProperty(Collections.singletonMap(Item.READ, Collections.singleton("alice")))); + */ + WebClient wc = createWebClient(); + wc.login("alice"); + HtmlPage page = wc.getPage(upstream, "configure"); + HtmlForm config = page.getFormByName("config"); + config.getButtonByCaption("Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly + page.getAnchorByText("Build other projects").click(); + HtmlTextInput childProjects = config.getInputByName("buildTrigger.childProjects"); + childProjects.setValueAttribute("downstream"); + try { + submit(config); + fail(); + } catch (FailingHttpStatusCodeException x) { + assertEquals(403, x.getStatusCode()); + } + assertEquals(Collections.emptyList(), upstream.getDownstreamProjects()); + } + } From 7541e83cc9812afc2b464f0a3254a2453da53f4c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 11 Feb 2014 17:29:05 -0500 Subject: [PATCH 15/15] [SECURITY-76] [SECURITY-88] Adjust test to policy changes. --- core/src/test/java/hudson/markup/MyspacePolicyTest.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/src/test/java/hudson/markup/MyspacePolicyTest.java b/core/src/test/java/hudson/markup/MyspacePolicyTest.java index cb1a63e2bb..8b8db118d4 100644 --- a/core/src/test/java/hudson/markup/MyspacePolicyTest.java +++ b/core/src/test/java/hudson/markup/MyspacePolicyTest.java @@ -34,9 +34,7 @@ public class MyspacePolicyTest extends Assert { assertIntact("
header
something
"); assertIntact("

title

blurb
"); - assertIntact(""); - assertIntact(""); - assertReject("javascript",""); + assertReject("iframe", ""); assertReject("script",""); assertReject("script",""); @@ -47,7 +45,7 @@ public class MyspacePolicyTest extends Assert { assertIntact("
inline CSS
"); assertIntact("
"); - assertIntact("
"); + assertReject("sun.com", "
"); } private void assertIntact(String input) {