Skip to content

Commit

Permalink
[SECURITY-1266] Don't execute AST transforms in validate/translate
Browse files Browse the repository at this point in the history
  • Loading branch information
abayer committed Jan 2, 2019
1 parent b30d35b commit 6d7884d
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 8 deletions.
Expand Up @@ -42,14 +42,15 @@ import org.jenkinsci.plugins.pipeline.modeldefinition.ASTSchema
import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTPipelineDef
import org.jenkinsci.plugins.pipeline.modeldefinition.ast.ModelASTStep
import org.jenkinsci.plugins.pipeline.modeldefinition.validator.DeclarativeValidatorContributor
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox
import org.jenkinsci.plugins.workflow.cps.CpsThread
import org.jenkinsci.plugins.workflow.cps.GroovyShellDecorator

import java.security.CodeSource
import java.security.cert.Certificate

import static groovy.lang.GroovyShell.DEFAULT_CODE_BASE
import static org.codehaus.groovy.control.Phases.CANONICALIZATION
import static org.codehaus.groovy.control.Phases.CONVERSION

/**
* Utilities for converting from/to {@link ModelASTPipelineDef} and raw Pipeline script.
Expand Down Expand Up @@ -133,7 +134,7 @@ class Converter {
}

private static CompilerConfiguration makeCompilerConfiguration() {
CompilerConfiguration cc = new CompilerConfiguration()
CompilerConfiguration cc = GroovySandbox.createBaseCompilerConfiguration()

ImportCustomizer ic = new ImportCustomizer()
ic.addStarImports(NonCPS.class.getPackage().getName())
Expand Down Expand Up @@ -166,9 +167,9 @@ class Converter {
model[0] = new ModelParser(source, enabledOptionalValidators).parse(true)
}
}
}, CANONICALIZATION)
}, CONVERSION)

cu.compile(CANONICALIZATION)
cu.compile(CONVERSION)

return model[0]
}
Expand All @@ -195,9 +196,9 @@ class Converter {
model[0] = new ModelParser(source, enabledOptionalValidators).parsePlainSteps(source.AST)
}
}
}, CANONICALIZATION)
}, CONVERSION)

cu.compile(CANONICALIZATION)
cu.compile(CONVERSION)

return model[0]
}
Expand Down
Expand Up @@ -33,13 +33,15 @@
import org.jenkinsci.plugins.pipeline.modeldefinition.model.BuildCondition;
import org.jenkinsci.plugins.pipeline.modeldefinition.model.Tools;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

import java.net.URL;
import java.util.Collections;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;

public class ModelConverterActionTest extends AbstractModelDefTest {
Expand Down Expand Up @@ -243,4 +245,34 @@ public void testInvalidScriptContentsInJson() throws Exception {
assertTrue("Errors array (" + topErrors.toString(2) + ") didn't contain expected error '" + expectedError + "'",
foundExpectedErrorInJSON(topErrors, expectedError));
}

@Issue("SECURITY-1266")
@Test
public void testLocalASTTransformInCompilation() throws Exception {
final String rawJenkinsfile = fileContentsFromResources("localASTTransformInCompilation.groovy", true);
// TODO: Get a better approach for skipping JSON-specific errors
if (rawJenkinsfile != null) {

JenkinsRule.WebClient wc = j.createWebClient();
WebRequest req = new WebRequest(new URL(wc.getContextPath() + ModelConverterAction.PIPELINE_CONVERTER_URL + "/validateJenkinsfile"), HttpMethod.POST);

assertNotNull(rawJenkinsfile);

NameValuePair pair = new NameValuePair("jenkinsfile", rawJenkinsfile);
req.setRequestParameters(Collections.singletonList(pair));

String rawResult = wc.getPage(req).getWebResponse().getContentAsString();
assertNotNull(rawResult);

JSONObject result = JSONObject.fromObject(rawResult);
// TODO: Change this when we get proper JSON errors causing HTTP error codes
assertEquals("Full result doesn't include status - " + result.toString(2), "ok", result.getString("status"));
JSONObject resultData = result.getJSONObject("data");
assertNotNull(resultData);
assertEquals("Result wasn't a failure - " + result.toString(2), "failure", resultData.getString("result"));

assertNull(j.jenkins.getItem("should-not-exist"));
}
}

}
@@ -0,0 +1,44 @@
/*
* The MIT License
*
* Copyright (c) 2016, 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.
*/

import groovy.transform.*
import jenkins.model.Jenkins
import org.jenkinsci.plugins.workflow.job.WorkflowJob

@ASTTest(value={ assert Jenkins.get().createProject(WorkflowJob.class, "should-not-exist") })
@Field int x

pipeline {
agent none
stages {
stage("foo") {
steps {
echo "hello"
}
}
}
}



4 changes: 2 additions & 2 deletions pom.xml
Expand Up @@ -85,12 +85,12 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-cps</artifactId>
<version>2.58</version>
<version>2.61.1</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.46</version>
<version>1.50</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down

0 comments on commit 6d7884d

Please sign in to comment.