From 47e943c3535aee1b579f8281bdf37f8f903ef922 Mon Sep 17 00:00:00 2001 From: Dominik Stadler Date: Wed, 8 Mar 2023 17:34:21 +0100 Subject: [PATCH] [SECURITY-3061] --- .../plugins/jacoco/report/AbstractReport.java | 15 ++++++++-- .../plugins/jacoco/report/ClassReport.java | 3 +- .../plugins/jacoco/report/MethodReport.java | 12 ++++---- .../plugins/jacoco/report/PackageReport.java | 10 +++---- .../jacoco/report/SourceFileReport.java | 8 +++--- .../jacoco/report/AbstractReportTest.java | 8 ++++-- .../jacoco/report/AggregatedReportTest.java | 27 ++++++++++++------ .../jacoco/report/ClassReportTest.java | 28 +++++++++++-------- .../jacoco/report/CoverageReportTest.java | 12 +++++--- .../jacoco/report/MethodReportTest.java | 22 +++++++++------ .../jacoco/report/PackageReportTest.java | 18 ++++++++---- .../jacoco/report/SourceFileReportTest.java | 5 ++++ 12 files changed, 109 insertions(+), 59 deletions(-) diff --git a/src/main/java/hudson/plugins/jacoco/report/AbstractReport.java b/src/main/java/hudson/plugins/jacoco/report/AbstractReport.java index 6a9cbe01..d5d304a0 100644 --- a/src/main/java/hudson/plugins/jacoco/report/AbstractReport.java +++ b/src/main/java/hudson/plugins/jacoco/report/AbstractReport.java @@ -31,7 +31,18 @@ public String getName() { } public void setName(String name) { - this.name = name; + this.name = sanitizeName(name); + } + + protected static String sanitizeName(String name) { + // sanitize names contained in .class files + return name + .replace(':', '_') + .replace(';', '_') + .replace('&', '_') + .replace('%', '_') + .replace('<', '_') + .replace('>', '_'); } public String getDisplayName() { @@ -72,5 +83,5 @@ public SELF getPreviousResult() { public Run getBuild() { return parent.getBuild(); } - + } diff --git a/src/main/java/hudson/plugins/jacoco/report/ClassReport.java b/src/main/java/hudson/plugins/jacoco/report/ClassReport.java index 5064489a..c192082e 100644 --- a/src/main/java/hudson/plugins/jacoco/report/ClassReport.java +++ b/src/main/java/hudson/plugins/jacoco/report/ClassReport.java @@ -15,9 +15,10 @@ public final class ClassReport extends AggregatedReport - AbstractReport public final class MethodReport extends AggregatedReport { - + private IMethodCoverage methodCov; - + @Override public String printFourCoverageColumns() { StringBuilder buf = new StringBuilder(); @@ -32,10 +32,10 @@ public String printFourCoverageColumns() { //logger.log(Level.INFO, "Printing Ratio cells within MethodReport."); return buf.toString(); } - + @Override public void add(SourceFileReport child) { - String newChildName = child.getName().replaceAll(this.getName() + ".", ""); + String newChildName = child.getName().replace(this.getName() + ".", ""); child.setName(newChildName); getChildren().put(child.getName(), child); //logger.log(Level.INFO, "SourceFileReport"); @@ -45,11 +45,11 @@ public void add(SourceFileReport child) { public boolean hasClassCoverage() { return false; } - + public void setSrcFileInfo(IMethodCoverage methodCov) { this.methodCov = methodCov; } - + public void printHighlightedSrcFile(Writer output) { new SourceAnnotator(getParent().getSourceFilePath()).printHighlightedSrcFile(methodCov,output); } diff --git a/src/main/java/hudson/plugins/jacoco/report/PackageReport.java b/src/main/java/hudson/plugins/jacoco/report/PackageReport.java index 85b9d50c..386f2e4e 100644 --- a/src/main/java/hudson/plugins/jacoco/report/PackageReport.java +++ b/src/main/java/hudson/plugins/jacoco/report/PackageReport.java @@ -18,17 +18,17 @@ public String getName() { @Override public void setName(String name) { - super.setName(name.replaceAll("/", ".")); + super.setName(name.replace('/', '.')); } - + @Override public void add(ClassReport child) { - String newChildName = child.getName().replaceAll(this.getName() + ".", ""); + String newChildName = child.getName().replace(this.getName() + ".", ""); child.setName(newChildName); this.getChildren().put(child.getName(), child); //logger.log(Level.INFO, "PackageReport"); } - + //private static final Logger logger = Logger.getLogger(CoverageObject.class.getName()); - + } diff --git a/src/main/java/hudson/plugins/jacoco/report/SourceFileReport.java b/src/main/java/hudson/plugins/jacoco/report/SourceFileReport.java index ff41c3e3..fffbab44 100644 --- a/src/main/java/hudson/plugins/jacoco/report/SourceFileReport.java +++ b/src/main/java/hudson/plugins/jacoco/report/SourceFileReport.java @@ -5,12 +5,12 @@ * @author Kohsuke Kawaguchi */ public final class SourceFileReport extends AbstractReport { - + @Override public void setName(String name) { - super.setName(name.replaceAll("/", ".")); + super.setName(name.replace('/', '.')); //logger.log(Level.INFO, "SourceFileReport"); } - + //private static final Logger logger = Logger.getLogger(SourceFileReport.class.getName()); -} +} diff --git a/src/test/java/hudson/plugins/jacoco/report/AbstractReportTest.java b/src/test/java/hudson/plugins/jacoco/report/AbstractReportTest.java index b0767ea5..d0d38e6b 100644 --- a/src/test/java/hudson/plugins/jacoco/report/AbstractReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/AbstractReportTest.java @@ -17,7 +17,7 @@ public void test() throws Exception { // abstract class but not abstract method to override }; assertNotNull(report); - + report.setParent(new ClassReport()); report.getParent().setParent(new PackageReport()); @@ -33,7 +33,11 @@ public void test() throws Exception { report.setName("testname"); assertEquals("testname", report.getName()); assertEquals("testname", report.getDisplayName()); - + + report.setName("myname/&:<>2%;"); + assertEquals("myname/____2__", report.getName()); + assertEquals("myname/____2__", report.getDisplayName()); + // TODO: cause NPEs, did not find out how to test this without a full jenkins-test //assertNull(report.getPreviousResult()); //CoverageElement cv = new CoverageElement(); diff --git a/src/test/java/hudson/plugins/jacoco/report/AggregatedReportTest.java b/src/test/java/hudson/plugins/jacoco/report/AggregatedReportTest.java index 1148f0b7..df905e71 100644 --- a/src/test/java/hudson/plugins/jacoco/report/AggregatedReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/AggregatedReportTest.java @@ -11,10 +11,10 @@ public class AggregatedReportTest { public void testSetFailed() throws Exception { AggregatedReport report = new AggregatedReport() { }; - + assertEquals(0, report.getChildren().size()); assertFalse(report.hasChildren()); - + MethodReport child = new MethodReport(); child.setName("testmethod"); report.add(child); @@ -22,25 +22,29 @@ public void testSetFailed() throws Exception { assertTrue(report.hasChildren()); assertFalse(report.hasChildrenClassCoverage()); assertFalse(report.hasChildrenLineCoverage()); - + report.setParent(new PackageReport()); assertNotNull(report.getParent()); - + assertNull(report.getDynamic("test", null, null)); assertNotNull(report.getDynamic("testmethod", null, null)); - + report.setFailed(); - + child.getLineCoverage().accumulate(0, 3); assertTrue(report.hasChildrenLineCoverage()); child.getClassCoverage().accumulate(0, 3); assertFalse("For method children it's always false", report.hasChildrenClassCoverage()); + + report.setName("myname/&:<>2%;"); + assertEquals("myname/____2__", report.getName()); + assertEquals("myname/____2__", report.getDisplayName()); } - + @Test public void testClassCoverage() { - AggregatedReport packageReport = + AggregatedReport packageReport = new AggregatedReport() { }; @@ -52,8 +56,13 @@ public void testClassCoverage() { assertFalse(packageReport.hasChildrenLineCoverage()); classChild.getClassCoverage().accumulate(0, 3); - + assertTrue(packageReport.hasChildrenClassCoverage()); assertFalse(packageReport.hasChildrenLineCoverage()); + + classChild = new ClassReport(); + classChild.setName("testclass/pkg"); + packageReport.add(classChild); + assertEquals("testclass.pkg", classChild.getName()); } } diff --git a/src/test/java/hudson/plugins/jacoco/report/ClassReportTest.java b/src/test/java/hudson/plugins/jacoco/report/ClassReportTest.java index 25cfeb0e..247939f7 100644 --- a/src/test/java/hudson/plugins/jacoco/report/ClassReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/ClassReportTest.java @@ -11,46 +11,50 @@ public class ClassReportTest { @Test - public void testName() throws Exception { + public void testName() { ClassReport report = new ClassReport(); report.setName("testname"); assertEquals("testname", report.getName()); report.setName("test/name/1"); assertEquals("test.name.1", report.getName()); + + report.setName("myname/&:<>2%;"); + assertEquals("myname.____2__", report.getName()); + assertEquals("myname.____2__", report.getDisplayName()); } - + @Test - public void testChildren() throws Exception { + public void testChildren() { ClassReport report = new ClassReport(); - + assertEquals(0, report.getChildren().size()); MethodReport child = new MethodReport(); child.setName("testname"); report.add(child); assertEquals(1, report.getChildren().size()); } - + @Test - public void testSourceFile() throws Exception { + public void testSourceFile() { ClassReport report = new ClassReport(); report.setSrcFileInfo(null, "some/path"); assertEquals(new File("some/path"), report.getSourceFilePath()); } - + @Test - public void testPrint() throws Exception { + public void testPrint() { ClassReport report = new ClassReport(); report.setSrcFileInfo(null, "some/path"); - + StringWriter writer = new StringWriter(); report.printHighlightedSrcFile(writer); - + String string = writer.toString(); assertEquals("ERROR: Error while reading the sourcefile!", string); } - + @Test - public void testToString() throws Exception { + public void testToString() { ClassReport report = new ClassReport(); assertNotNull(report.toString()); } diff --git a/src/test/java/hudson/plugins/jacoco/report/CoverageReportTest.java b/src/test/java/hudson/plugins/jacoco/report/CoverageReportTest.java index b4229ae0..c7bcfa90 100644 --- a/src/test/java/hudson/plugins/jacoco/report/CoverageReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/CoverageReportTest.java @@ -11,19 +11,23 @@ public class CoverageReportTest { @Test - public void testGetBuild() throws Exception { + public void testGetBuild() { CoverageReport report = new CoverageReport(action, new ExecutionFileLoader()); assertNull(report.getBuild()); } @Test - public void testName() throws Exception { + public void testName() { CoverageReport report = new CoverageReport(action, new ExecutionFileLoader()); assertEquals("Jacoco", report.getName()); + + report.setName("myname/&:<>2%;"); + assertEquals("myname/____2__", report.getName()); + assertEquals("myname/____2__", report.getDisplayName()); } @Test - public void testDoJaCoCoExec() throws Exception { + public void testDoJaCoCoExec() { CoverageReport report = new CoverageReport(action, new ExecutionFileLoader()); assertNotNull(report); // TODO: how to simulate JaCoCoBuildAction without full Jenkins test-framework? @@ -31,7 +35,7 @@ public void testDoJaCoCoExec() throws Exception { } @Test - public void testThresholds() throws Exception { + public void testThresholds() { CoverageReport report = new CoverageReport(action, new ExecutionFileLoader()); report.setThresholds(new JacocoHealthReportThresholds()); } diff --git a/src/test/java/hudson/plugins/jacoco/report/MethodReportTest.java b/src/test/java/hudson/plugins/jacoco/report/MethodReportTest.java index 19f72dea..9006413a 100644 --- a/src/test/java/hudson/plugins/jacoco/report/MethodReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/MethodReportTest.java @@ -12,47 +12,53 @@ public class MethodReportTest { public void testMissingFile() { MethodReport report = new MethodReport(); assertFalse(report.hasClassCoverage()); - + report.setSrcFileInfo(null); - + ClassReport p = new ClassReport(); p.setSrcFileInfo(null, "some/path"); report.setParent(p); - + StringWriter writer = new StringWriter(); report.printHighlightedSrcFile(writer); String string = writer.toString(); assertEquals("ERROR: Error while reading the sourcefile!", string); + + report.setName("myname/&:<>2%;"); + assertEquals("myname/____2__", report.getName()); + assertEquals("myname/____2__", report.getDisplayName()); } @Test - public void testPrint() throws Exception { + public void testPrint() { MethodReport report = new MethodReport(); assertNotNull(report.printFourCoverageColumns()); } @Test - public void testChildren() throws Exception { + public void testChildren() { MethodReport report = new MethodReport(); report.setName("pkg"); - + assertEquals(0, report.getChildren().size()); SourceFileReport child = new SourceFileReport(); child.setName("testname"); report.add(child); + assertEquals("testname", child.getName()); assertEquals(1, report.getChildren().size()); assertEquals("testname", report.getChildren().values().iterator().next().getName()); } @Test - public void testChildrenRemovePkgName() throws Exception { + public void testChildrenRemovePkgName() { MethodReport report = new MethodReport(); report.setName("pkg"); - + assertEquals(0, report.getChildren().size()); SourceFileReport child = new SourceFileReport(); child.setName("pkg.testname"); report.add(child); + assertEquals("testname", child.getName()); assertEquals(1, report.getChildren().size()); assertEquals("testname", report.getChildren().values().iterator().next().getName()); } diff --git a/src/test/java/hudson/plugins/jacoco/report/PackageReportTest.java b/src/test/java/hudson/plugins/jacoco/report/PackageReportTest.java index bef7b6e9..0149e8e5 100644 --- a/src/test/java/hudson/plugins/jacoco/report/PackageReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/PackageReportTest.java @@ -8,9 +8,9 @@ public class PackageReportTest { @Test - public void testName() throws Exception { + public void testName() { PackageReport report = new PackageReport(); - + report.setName(""); assertEquals("(default)", report.getName()); @@ -19,30 +19,36 @@ public void testName() throws Exception { report.setName("test/name/1"); assertEquals("test.name.1", report.getName()); + + report.setName("myname/&:<>2%;"); + assertEquals("myname.____2__", report.getName()); + assertEquals("myname.____2__", report.getDisplayName()); } @Test - public void testChildren() throws Exception { + public void testChildren() { PackageReport report = new PackageReport(); report.setName("pkg"); - + assertEquals(0, report.getChildren().size()); ClassReport child = new ClassReport(); child.setName("testname"); report.add(child); + assertEquals("testname", child.getName()); assertEquals(1, report.getChildren().size()); assertEquals("testname", report.getChildren().values().iterator().next().getName()); } @Test - public void testChildrenRemovePkgName() throws Exception { + public void testChildrenRemovePkgName() { PackageReport report = new PackageReport(); report.setName("pkg"); - + assertEquals(0, report.getChildren().size()); ClassReport child = new ClassReport(); child.setName("pkg.testname"); report.add(child); + assertEquals("testname", child.getName()); assertEquals(1, report.getChildren().size()); assertEquals("testname", report.getChildren().values().iterator().next().getName()); } diff --git a/src/test/java/hudson/plugins/jacoco/report/SourceFileReportTest.java b/src/test/java/hudson/plugins/jacoco/report/SourceFileReportTest.java index 94a027a7..92dd346c 100644 --- a/src/test/java/hudson/plugins/jacoco/report/SourceFileReportTest.java +++ b/src/test/java/hudson/plugins/jacoco/report/SourceFileReportTest.java @@ -12,5 +12,10 @@ public void test() { SourceFileReport report = new SourceFileReport(); report.setName("myname"); assertEquals("myname", report.getName()); + report.setName("myname/2"); + assertEquals("myname.2", report.getName()); + report.setName("myname/&:<>2%;"); + assertEquals("myname.____2__", report.getName()); + assertEquals("myname.____2__", report.getDisplayName()); } }