Skip to content

Commit 48548ae

Browse files
jimklimovjimklimov
authored
Merge branch 'master' into JENKINS-69731-scm-BRANCH_NAME
2 parents d039e50 + b44ce04 commit 48548ae

File tree

3 files changed

+142
-9
lines changed

3 files changed

+142
-9
lines changed

‎pom.xml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
<parent>
2929
<groupId>org.jenkins-ci.plugins</groupId>
3030
<artifactId>plugin</artifactId>
31-
<version>4.44</version>
31+
<version>4.50</version>
3232
<relativePath/>
3333
</parent>
3434
<groupId>io.jenkins.plugins</groupId>
@@ -44,7 +44,7 @@
4444
</license>
4545
</licenses>
4646
<scm>
47-
<connection>scm:git:git://github.com/${gitHubRepo}.git</connection>
47+
<connection>scm:git:https://github.com/${gitHubRepo}.git</connection>
4848
<developerConnection>scm:git:[email protected]:${gitHubRepo}.git</developerConnection>
4949
<url>https://github.com/${gitHubRepo}</url>
5050
<tag>${scmTag}</tag>
@@ -63,15 +63,15 @@
6363
</pluginRepositories>
6464
<properties>
6565
<changelist>999999-SNAPSHOT</changelist>
66-
<jenkins.version>2.289.3</jenkins.version>
66+
<jenkins.version>2.346.3</jenkins.version>
6767
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
6868
</properties>
6969
<dependencyManagement>
7070
<dependencies>
7171
<dependency>
7272
<groupId>io.jenkins.tools.bom</groupId>
73-
<artifactId>bom-2.289.x</artifactId>
74-
<version>1500.ve4d05cd32975</version>
73+
<artifactId>bom-2.346.x</artifactId>
74+
<version>1678.vc1feb_6a_3c0f1</version>
7575
<scope>import</scope>
7676
<type>pom</type>
7777
</dependency>
@@ -82,7 +82,7 @@
8282
<dependency>
8383
<groupId>org.apache.ivy</groupId>
8484
<artifactId>ivy</artifactId>
85-
<version>2.5.0</version>
85+
<version>2.5.1</version>
8686
</dependency>
8787

8888
<!-- required plugins -->

‎src/main/java/org/jenkinsci/plugins/workflow/libs/LibraryStep.java

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
import java.util.logging.Logger;
6060
import edu.umd.cs.findbugs.annotations.CheckForNull;
6161
import edu.umd.cs.findbugs.annotations.NonNull;
62+
import groovy.lang.MissingPropertyException;
6263
import javax.inject.Inject;
6364
import jenkins.model.Jenkins;
6465
import jenkins.scm.impl.SingleSCMSource;
@@ -75,6 +76,8 @@
7576
import org.jenkinsci.plugins.workflow.steps.StepContextParameter;
7677
import org.kohsuke.accmod.Restricted;
7778
import org.kohsuke.accmod.restrictions.DoNotUse;
79+
import org.kohsuke.groovy.sandbox.GroovyInterceptor;
80+
import org.kohsuke.groovy.sandbox.impl.Checker;
7881
import org.kohsuke.stapler.AncestorInPath;
7982
import org.kohsuke.stapler.DataBoundConstructor;
8083
import org.kohsuke.stapler.DataBoundSetter;
@@ -270,11 +273,15 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
270273
if (clazz != null) {
271274
// Field access?
272275
try {
273-
// not doing a Whitelist check since GroovyClassLoaderWhitelist would be allowing it anyway
276+
if (isSandboxed()) {
277+
return Checker.checkedGetAttribute(loadClass(prefix + clazz), false, false, property);
278+
}
274279
return loadClass(prefix + clazz).getField(property).get(null);
275-
} catch (NoSuchFieldException x) {
280+
} catch (MissingPropertyException | NoSuchFieldException x) {
276281
// guessed wrong
277-
} catch (IllegalAccessException x) {
282+
} catch (SecurityException x) {
283+
throw x;
284+
} catch (Throwable x) {
278285
throw new GroovyRuntimeException(x);
279286
}
280287
}
@@ -284,6 +291,8 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
284291
loadClass(prefix + fullClazz);
285292
// OK, class really exists, stash it and await methods
286293
return new LoadedClasses(library, trusted, changelog, prefix, fullClazz, srcUrl);
294+
} else if (clazz != null) {
295+
throw new MissingPropertyException(property, loadClass(prefix + clazz));
287296
} else {
288297
// Still selecting package components.
289298
return new LoadedClasses(library, trusted, changelog, prefix + property + '.', null, srcUrl);
@@ -293,13 +302,43 @@ public static final class LoadedClasses extends GroovyObjectSupport implements S
293302
@Override public Object invokeMethod(String name, Object _args) {
294303
Class<?> c = loadClass(prefix + clazz);
295304
Object[] args = _args instanceof Object[] ? (Object[]) _args : new Object[] {_args}; // TODO why does Groovy not just pass an Object[] to begin with?!
305+
if (isSandboxed()) {
306+
try {
307+
if (name.equals("new")) {
308+
return Checker.checkedConstructor(c, args);
309+
} else {
310+
return Checker.checkedStaticCall(c, name, args);
311+
}
312+
} catch (SecurityException x) {
313+
throw x;
314+
} catch (Throwable x) {
315+
throw new GroovyRuntimeException(x);
316+
}
317+
}
296318
if (name.equals("new")) {
297319
return InvokerHelper.invokeConstructorOf(c, args);
298320
} else {
299321
return InvokerHelper.invokeStaticMethod(c, name, args);
300322
}
301323
}
302324

325+
/**
326+
* Check whether the current thread has at least one active {@link GroovyInterceptor}.
327+
* <p>
328+
* Typically, {@code GroovyClassLoaderWhitelist} will allow access to everything defined in a class in a
329+
* library, but there are some synthetic constructors, fields, and methods which should not be accessible.
330+
* <p>
331+
* As a result, when getting properties or invoking methods using this class, we need to apply sandbox
332+
* protection if the Pipeline code performing the operation is sandbox-transformed. Unfortunately, it is
333+
* difficult to detect that case specifically, so we instead intercept all calls if the Pipeline itself is
334+
* sandboxed. This results in a false positive {@code RejectedAccessException} being thrown if a trusted
335+
* library uses the {@code library} step and tries to access static fields or methods that are not permitted to
336+
* be used in the sandbox.
337+
*/
338+
private static boolean isSandboxed() {
339+
return !GroovyInterceptor.getApplicableInterceptors().isEmpty();
340+
}
341+
303342
// TODO putProperty for static field set
304343

305344
private Class<?> loadClass(String name) {

‎src/test/java/org/jenkinsci/plugins/workflow/libs/LibraryStepTest.java

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@
5151
import org.junit.Test;
5252
import static org.junit.Assert.*;
5353
import org.junit.ClassRule;
54+
import org.junit.Ignore;
5455
import org.junit.Rule;
5556
import org.jvnet.hudson.test.BuildWatcher;
5657
import org.jvnet.hudson.test.Issue;
@@ -124,6 +125,99 @@ public class LibraryStepTest {
124125
r.assertLogContains("using constant vs. constant", b);
125126
}
126127

128+
@Test public void missingProperty() throws Exception {
129+
sampleRepo.init();
130+
sampleRepo.write("src/some/pkg/MyClass.groovy", "package some.pkg; class MyClass { }");
131+
sampleRepo.git("add", "src");
132+
sampleRepo.git("commit", "--message=init");
133+
Folder f = r.jenkins.createProject(Folder.class, "f");
134+
f.getProperties().add(new FolderLibraries(Collections.singletonList(new LibraryConfiguration("stuff", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))));
135+
WorkflowJob p = f.createProject(WorkflowJob.class, "p");
136+
p.setDefinition(new CpsFlowDefinition(
137+
"def lib = library 'stuff@master'\n" +
138+
"lib.some.pkg.MyClass.no_field_with_this_name\n" , true));
139+
WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p);
140+
r.assertLogContains("MissingPropertyException: No such property: no_field_with_this_name for class: some.pkg.MyClass", b);
141+
}
142+
143+
@Test public void reflectionInLoadedClassesIsIntercepted() throws Exception {
144+
sampleRepo.init();
145+
sampleRepo.write("src/some/pkg/MyThread.groovy", "package some.pkg; class MyThread extends Thread { }");
146+
sampleRepo.git("add", "src");
147+
sampleRepo.git("commit", "--message=init");
148+
Folder f = r.jenkins.createProject(Folder.class, "f");
149+
f.getProperties().add(new FolderLibraries(Collections.singletonList(new LibraryConfiguration("stuff", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))));
150+
WorkflowJob p = f.createProject(WorkflowJob.class, "p");
151+
p.setDefinition(new CpsFlowDefinition(
152+
"def lib = library 'stuff@master'\n" +
153+
"catchError() { lib.some.pkg.MyThread.new(null) }\n" +
154+
"catchError() { lib.some.pkg.MyThread.__$stMC }\n" +
155+
"catchError() { lib.some.pkg.MyThread.$getCallSiteArray() }\n" , true));
156+
WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p);
157+
r.assertLogContains("Rejecting illegal call to synthetic constructor", b);
158+
r.assertLogContains("staticField some.pkg.MyThread __$stMC", b);
159+
r.assertLogContains("staticMethod some.pkg.MyThread $getCallSiteArray", b);
160+
}
161+
162+
@Issue("SECURITY-2824")
163+
@Test public void constructorInvocationInLoadedClassesIsIntercepted() throws Exception {
164+
sampleRepo.init();
165+
sampleRepo.write("src/pkg/Superclass.groovy",
166+
"package pkg;\n" +
167+
"class Superclass { Superclass(String x) { } }\n");
168+
sampleRepo.write("src/pkg/Subclass.groovy",
169+
"package pkg;\n" +
170+
"class Subclass extends Superclass {\n" +
171+
" def wrapper\n" +
172+
" Subclass() { super('secret.key'); def $cw = $cw; wrapper = $cw }\n" +
173+
"}\n");
174+
sampleRepo.write("src/pkg/MyFile.groovy",
175+
"package pkg;\n" +
176+
"class MyFile extends File {\n" +
177+
" MyFile(String path) {\n" +
178+
" super(path)\n" +
179+
" }\n" +
180+
"}\n");
181+
sampleRepo.git("add", "src");
182+
sampleRepo.git("commit", "--message=init");
183+
Folder f = r.jenkins.createProject(Folder.class, "f");
184+
f.getProperties().add(new FolderLibraries(Collections.singletonList(new LibraryConfiguration("stuff", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))))));
185+
WorkflowJob p = f.createProject(WorkflowJob.class, "p");
186+
p.setDefinition(new CpsFlowDefinition(
187+
"def lib = library 'stuff@master'\n" +
188+
"def wrapper = lib.pkg.Subclass.new().wrapper\n" +
189+
"def file = lib.pkg.MyFile.new(wrapper, 'unused')\n" +
190+
"echo(/${[file, file.class]}/)", true));
191+
WorkflowRun b = r.buildAndAssertStatus(Result.FAILURE, p);
192+
r.assertLogContains("Rejecting illegal call to synthetic constructor: private pkg.MyFile", b);
193+
}
194+
195+
@Ignore("Trusted libraries should never get a RejectedAccessException, but this case should be uncommon and is difficult to handle more precisely")
196+
@Test public void falsePositiveRejectedAccessExceptionInTrustedLibrary() throws Exception {
197+
sampleRepo.init();
198+
sampleRepo.git("branch", "myBranch");
199+
sampleRepo.write("vars/doStuff.groovy",
200+
"def call() {\n" +
201+
" def lib = library('stuff2@myBranch')\n" +
202+
" lib.some.pkg.MyClass.$getCallSiteArray()\n" +
203+
"}\n");
204+
sampleRepo.git("add", ".");
205+
sampleRepo.git("commit", "--message=init");
206+
sampleRepo.git("checkout", "myBranch");
207+
sampleRepo.write("src/some/pkg/MyClass.groovy", "package some.pkg; class MyClass { }");
208+
sampleRepo.git("add", ".");
209+
sampleRepo.git("commit", "--message=myBranch");
210+
GlobalLibraries.get().setLibraries(Arrays.asList(
211+
new LibraryConfiguration("stuff", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true))),
212+
new LibraryConfiguration("stuff2", new SCMSourceRetriever(new GitSCMSource(null, sampleRepo.toString(), "", "*", "", true)))));
213+
WorkflowJob p = r.createProject(WorkflowJob.class, "p");
214+
p.setDefinition(new CpsFlowDefinition(
215+
"@Library('stuff@master')\n" +
216+
"import doStuff\n" +
217+
"doStuff()\n", true));
218+
WorkflowRun b = r.buildAndAssertSuccess(p);
219+
}
220+
127221
@Test public void classesFromWrongPlace() throws Exception {
128222
sampleRepo.init();
129223
sampleRepo.write("src/some/pkg/Lib.groovy", "package some.pkg; class Lib {static void m() {}}");

0 commit comments

Comments
 (0)