Skip to content

Commit 40c6d35

Browse files
author
Mark Robinson
committed
Prevent path traversal attacks in run parameters
1 parent 630efa2 commit 40c6d35

File tree

2 files changed

+56
-8
lines changed

2 files changed

+56
-8
lines changed

src/main/java/org/commonwl/viewer/domain/CWLCollection.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.commons.io.FilenameUtils;
2929
import org.commonwl.viewer.services.DockerService;
3030
import org.commonwl.viewer.services.GitHubService;
31+
import org.commonwl.viewer.web.PathTraversalException;
3132
import org.eclipse.egit.github.core.RepositoryContents;
3233
import org.yaml.snakeyaml.Yaml;
3334

@@ -200,14 +201,16 @@ private String findMainWorkflow() {
200201

201202
// Store the in degree of each workflow
202203
Map<String, Integer> inDegrees = new HashMap<String, Integer>();
203-
for (String key : cwlDocs.keySet()) {
204-
inDegrees.put(key, 0);
204+
for (Map.Entry<String, JsonNode> doc : cwlDocs.entrySet()) {
205+
if (doc.getValue().get(CLASS).asText().equals(WORKFLOW)) {
206+
inDegrees.put(doc.getKey(), 0);
207+
}
205208
}
206209

207210
// Loop through documents and calculate in degrees
208211
for (Map.Entry<String, JsonNode> doc : cwlDocs.entrySet()) {
209212
JsonNode content = doc.getValue();
210-
if (content.get(CLASS).asText().equals(WORKFLOW)) {
213+
if (inDegrees.containsKey(doc.getKey())) {
211214
// Parse workflow steps and see whether other workflows are run
212215
JsonNode steps = content.get(STEPS);
213216
if (steps.getClass() == ArrayNode.class) {
@@ -254,8 +257,9 @@ private String findMainWorkflow() {
254257
/**
255258
* Gets the Workflow object for this collection of documents
256259
* @return A Workflow object representing the main workflow amongst the files added
260+
* @throws PathTraversalException If run path is outside the root Github directory
257261
*/
258-
public Workflow getWorkflow() {
262+
public Workflow getWorkflow() throws PathTraversalException {
259263
// Get the main workflow
260264
if (mainWorkflowKey == null) {
261265
mainWorkflowKey = findMainWorkflow();
@@ -282,13 +286,28 @@ public Workflow getWorkflow() {
282286
/**
283287
* Fill the step runtypes based on types of other documents
284288
* @param workflow The workflow model to set runtypes for
289+
* @throws PathTraversalException If run path is outside the root Github directory
285290
*/
286-
private void fillStepRunTypes(Workflow workflow) {
291+
private void fillStepRunTypes(Workflow workflow) throws PathTraversalException {
287292
Map<String, CWLStep> steps = workflow.getSteps();
288293
for (CWLStep step : steps.values()) {
289-
Path filePath = Paths.get(githubInfo.getPath()).resolve(step.getRun());
290-
JsonNode runDoc = cwlDocs.get(filePath.toString());
291-
step.setRunType(extractProcess(runDoc));
294+
String runParam = step.getRun();
295+
if (runParam != null) {
296+
JsonNode runDoc;
297+
if (runParam.startsWith("#")) {
298+
runDoc = cwlDocs.get(runParam.substring(1));
299+
} else {
300+
Path basePath = Paths.get(githubInfo.getPath());
301+
Path filePath = basePath.resolve(runParam);
302+
if (!filePath.startsWith(basePath)) {
303+
throw new PathTraversalException("Step run parameter path '" + filePath +
304+
"' is outside base path '" + basePath + "'");
305+
}
306+
runDoc = cwlDocs.get(filePath.toString());
307+
step.setRunType(extractProcess(runDoc));
308+
}
309+
step.setRunType(extractProcess(runDoc));
310+
}
292311
}
293312
}
294313

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.commonwl.viewer.web;
21+
22+
/**
23+
* Exception thrown when a potential path traversal attempt was prevented
24+
*/
25+
public class PathTraversalException extends Exception {
26+
public PathTraversalException(String message) {
27+
super(message);
28+
}
29+
}

0 commit comments

Comments
 (0)