Skip to content
This repository has been archived by the owner on Nov 19, 2024. It is now read-only.

Commit

Permalink
[SECURITY-1668]
Browse files Browse the repository at this point in the history
Co-authored-by: Federico Pellegrin <fede@evolware.org>
  • Loading branch information
2 people authored and daniel-beck committed Mar 3, 2020
1 parent fdee535 commit ea41b3f
Show file tree
Hide file tree
Showing 4 changed files with 96 additions and 5 deletions.
24 changes: 24 additions & 0 deletions src/main/java/hudson/plugins/cobertura/IOUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

import java.io.Closeable;

import com.google.common.base.Charsets;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import org.apache.commons.codec.binary.Hex;

/**
* General IO stream manipulation utilities.
*/
Expand All @@ -20,4 +25,23 @@ public static void closeQuietly(Closeable closeable) {
}
}
}

/**
* Sanitizes filename to prevent directory trasversals or other security threats
* Transforms every non alphanumeric character in its ascii number in the _XX format
*
* @param inputName the name to sanitize
* @return the sanitized string
*/
public static String sanitizeFilename(String inputName) {
Pattern p = Pattern.compile("[^a-zA-Z0-9-]");
Matcher m = p.matcher(inputName);
StringBuffer sb = new StringBuffer();
while (m.find()) {
String match = m.group();
m.appendReplacement(sb, "_" + Hex.encodeHexString(match.getBytes(Charsets.UTF_8)));
}
m.appendTail(sb);
return sb.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import hudson.FilePath;
import hudson.model.TaskListener;
import hudson.plugins.cobertura.IOUtils;
import hudson.plugins.cobertura.targets.CoveragePaint;
import hudson.remoting.VirtualChannel;
import jenkins.MasterToSlaveFileCallable;
Expand Down Expand Up @@ -114,7 +115,7 @@ public void paintSourceCode(File source, CoveragePaint paint, FilePath canvas) t
* {@inheritDoc}
*/
public Boolean invoke(File workspaceDir, VirtualChannel channel) throws IOException {
final List<File> trialPaths = new ArrayList<File>(sourcePaths.size());
final List<File> trialPaths = new ArrayList<>(sourcePaths.size());
for (String sourcePath : sourcePaths) {
final File trialPath = new File(sourcePath);
if (trialPath.exists()) {
Expand All @@ -134,7 +135,9 @@ public Boolean invoke(File workspaceDir, VirtualChannel channel) throws IOExcept
}
if (source.isFile()) {
try {
paintSourceCode(source, entry.getValue(), destination.child(entry.getKey()));
// sanitizing file name to avoid arbitrarily write
String sanitizedName = IOUtils.sanitizeFilename(entry.getKey());
paintSourceCode(source, entry.getValue(), destination.child(sanitizedName));
} catch (IOException e) {
// We made our best shot at generating painted source code,
// but alas, we failed. Log the error and continue. We
Expand Down
20 changes: 17 additions & 3 deletions src/main/java/hudson/plugins/cobertura/targets/CoverageResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import hudson.plugins.cobertura.Chartable;
import hudson.plugins.cobertura.CoberturaBuildAction;
import hudson.plugins.cobertura.CoverageChart;
import hudson.plugins.cobertura.IOUtils;
import hudson.plugins.cobertura.Ratio;
import hudson.util.Graph;
import hudson.util.TextFile;
Expand All @@ -29,6 +30,7 @@
import java.util.TreeMap;
import java.util.TreeSet;

import javax.annotation.Nullable;
import org.jfree.chart.JFreeChart;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
Expand Down Expand Up @@ -171,9 +173,16 @@ public void paint(int line, int hits, int branchHits, int branchTotal) {
*
* @return The file where the source file should be (if it exists)
*/
@Nullable
private File getSourceFile() {
if (hasPermission()) {
return new File(owner.getParent().getRootDir(), "cobertura/" + relativeSourcePath);
File sourceFile = new File(owner.getParent().getRootDir(), "cobertura/" + IOUtils.sanitizeFilename(relativeSourcePath));
if (sourceFile.exists()) {
return sourceFile;
} else {
// keep compatibility
return new File(owner.getParent().getRootDir(), "cobertura/" + relativeSourcePath);
}
}
return null;
}
Expand All @@ -185,7 +194,8 @@ private File getSourceFile() {
*/
public boolean isSourceFileAvailable() {
if (hasPermission()) {
return owner == owner.getParent().getLastSuccessfulBuild() && getSourceFile().exists();
File sourceFile = getSourceFile();
return owner == owner.getParent().getLastSuccessfulBuild() && sourceFile != null && sourceFile.exists();
}
return false;
}
Expand All @@ -202,7 +212,11 @@ public boolean hasPermission() {
public String getSourceFileContent() {
if (hasPermission()) {
try {
return new TextFile(getSourceFile()).read();
File sourceFile = getSourceFile();
if (sourceFile == null) {
return null;
}
return new TextFile(sourceFile).read();
} catch (IOException e) {
return null;
}
Expand Down
50 changes: 50 additions & 0 deletions src/test/java/hudson/plugins/cobertura/IOUtilsTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package hudson.plugins.cobertura;

import static org.junit.Assert.assertEquals;

import org.junit.Test;

public class IOUtilsTest {

@Test
public void testSanitizeRelativePathInUnix() {
String fileName = "../aaa/bbb";
String sanitizedName = IOUtils.sanitizeFilename(fileName);

assertEquals("_2e_2e_2faaa_2fbbb", sanitizedName);

fileName = "aaa/../bbb";
sanitizedName = IOUtils.sanitizeFilename(fileName);

assertEquals("aaa_2f_2e_2e_2fbbb", sanitizedName);
}

@Test
public void testSanitizeRelativePathInWindows() {
String fileName = "..\\aaa\\bbb";
String sanitizedName = IOUtils.sanitizeFilename(fileName);

assertEquals("_2e_2e_5caaa_5cbbb", sanitizedName);

fileName = "aaa\\..\\bbb";
sanitizedName = IOUtils.sanitizeFilename(fileName);
assertEquals( "aaa_5c_2e_2e_5cbbb", sanitizedName);
}


@Test
public void testSanitizeAbsolutePathInUnix() {
String fileName = "/aaa/bbb";
String sanitizedFileName = IOUtils.sanitizeFilename(fileName);

assertEquals("_2faaa_2fbbb", sanitizedFileName);
}

@Test
public void testSanitizeAbsolutePathInWindows() {
String fileName = "C:\\aaa\\bbb";
String sanitizedName = IOUtils.sanitizeFilename(fileName);

assertEquals("C_3a_5caaa_5cbbb", sanitizedName);
}
}

0 comments on commit ea41b3f

Please # to comment.