From 1962987bc4a317976d63959c0ed9b7121d928f0c Mon Sep 17 00:00:00 2001 From: Matthew DeTullio Date: Tue, 16 Feb 2016 17:32:34 -0500 Subject: [PATCH] Fix #766: Handle relative report paths with backticks outside basedir. It was possible to have an NPE when normalizing backticks on relative paths if the backticks moved the directory too far back. This change will handle invalid normalization of relative paths. Normalization is reattempted against the absolute path, and if it is still invalid then it is not given to the directory scanner, which does not accept null elements. --- .../plugins/cxx/utils/CxxReportSensor.java | 37 +++++----- .../CxxReportSensor_getReports_Test.java | 69 +++++++++++-------- 2 files changed, 62 insertions(+), 44 deletions(-) diff --git a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java index 0bc020f4e3..f59448d18f 100644 --- a/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java +++ b/sonar-cxx-plugin/src/main/java/org/sonar/plugins/cxx/utils/CxxReportSensor.java @@ -25,9 +25,6 @@ import java.util.List; import java.util.ArrayList; import java.util.Set; -import javax.annotation.Nullable; -import com.google.common.base.Function; -import com.google.common.collect.Lists; import org.apache.commons.io.FilenameUtils; import org.apache.tools.ant.DirectoryScanner; import org.sonar.api.batch.Sensor; @@ -152,24 +149,30 @@ public static List getReports(Settings settings, final File moduleBaseDir, List reports = new ArrayList<>(); - List includes = Arrays.asList(settings.getStringArray(reportPathPropertyKey)); - if (!includes.isEmpty()) { - includes = Lists.transform(includes, new Function() { - @Override - public String apply(@Nullable String path) { - if (path == null){ - return null; - } - path = FilenameUtils.normalize(path); - if (new File(path).isAbsolute()) { - return path; - } - return FilenameUtils.normalize(moduleBaseDir.getAbsolutePath() + File.separator + path); + List reportPaths = Arrays.asList(settings.getStringArray(reportPathPropertyKey)); + if (!reportPaths.isEmpty()) { + List includes = new ArrayList<>(); + for (String reportPath : reportPaths) { + // Normalization can return null if path is null, is invalid, or is a path with back-ticks outside known directory structure + String normalizedPath = FilenameUtils.normalize(reportPath); + if (normalizedPath != null && new File(normalizedPath).isAbsolute()) { + includes.add(normalizedPath); + continue; } - }); + + // Prefix with absolute module base dir, attempt normalization again -- can still get null here + normalizedPath = FilenameUtils.normalize(moduleBaseDir.getAbsolutePath() + File.separator + reportPath); + if (normalizedPath != null) { + includes.add(normalizedPath); + continue; + } + + CxxUtils.LOG.debug("Not a valid report path '{}'", reportPath); + } CxxUtils.LOG.debug("Normalized report includes to '{}'", includes); + // Includes array cannot contain null elements DirectoryScanner directoryScanner = new DirectoryScanner(); directoryScanner.setIncludes(includes.toArray(new String[includes.size()])); directoryScanner.scan(); diff --git a/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/utils/CxxReportSensor_getReports_Test.java b/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/utils/CxxReportSensor_getReports_Test.java index d0a876ff90..77ce7711f7 100644 --- a/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/utils/CxxReportSensor_getReports_Test.java +++ b/sonar-cxx-plugin/src/test/java/org/sonar/plugins/cxx/utils/CxxReportSensor_getReports_Test.java @@ -31,11 +31,9 @@ import org.apache.commons.io.FileUtils; import org.apache.commons.lang.StringUtils; -import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; -import org.sonar.api.batch.fs.FileSystem; import org.sonar.api.config.Settings; import org.sonar.plugins.cxx.TestUtils; @@ -43,27 +41,9 @@ public class CxxReportSensor_getReports_Test { private static final String REPORT_PATH_KEY = "sonar.cxx.cppcheck.reportPath"; - private class CxxReportSensorImpl extends CxxReportSensor { - - public CxxReportSensorImpl(Settings settings, FileSystem fs) { - super(settings, fs); - } - }; - @Rule public TemporaryFolder base = new TemporaryFolder(); - private CxxReportSensor sensor; - private Settings settings; - private FileSystem fs; - - @Before - public void init() { - settings = new Settings(); - fs = TestUtils.mockFileSystem(); - sensor = new CxxReportSensorImpl(settings, fs); - } - @Test public void getReports_patternMatching() throws java.io.IOException, java.lang.InterruptedException { Settings settings = new Settings(); @@ -75,6 +55,7 @@ public void getReports_patternMatching() throws java.io.IOException, java.lang.I examples.add(new String[]{"dir/A.ext", "dir/A.ext", "A.ext,dir/B.ext"}); examples.add(new String[]{"dir/../A.ext", "A.ext", "B.ext,dir/A.ext"}); examples.add(new String[]{"./A.ext", "A.ext", "B.ext"}); + examples.add(new String[]{"./A.ext", "A.ext", "B.ext"}); // empty examples.add(new String[]{"", "", ""}); // question mark glob @@ -94,7 +75,7 @@ public void getReports_patternMatching() throws java.io.IOException, java.lang.I setupExample(allpaths); settings.setProperty(REPORT_PATH_KEY, pattern); - reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertMatch(reports, match, example[0]); deleteExample(base.getRoot()); @@ -135,7 +116,7 @@ public void testAbsoluteInsideBasedir() throws IOException { Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString()); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(1, reports.size()); } @@ -147,7 +128,7 @@ public void testAbsoluteOutsideBasedir() { Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString()); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(1, reports.size()); } @@ -159,7 +140,7 @@ public void testAbsoluteOutsideBasedirWithGlobbing() { Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString()); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(2, reports.size()); } @@ -174,7 +155,7 @@ public void testAbsoluteOutsideBasedirAndRelative() throws IOException { Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + "," + relativeReport); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(2, reports.size()); } @@ -188,11 +169,14 @@ public void testAbsoluteOutsideBasedirWithGlobbingAndRelativeWithGlobbing() thro FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml")); FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml")); FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml")); + FileUtils.touch(new File(base.getRoot(), "some/reports/a")); + FileUtils.touch(new File(base.getRoot(), "some/reports/b")); + Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + ",**/*.xml"); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(7, reports.size()); } @@ -205,11 +189,42 @@ public void testAbsoluteOutsideBasedirWithGlobbingAndNestedRelativeWithGlobbing( FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml")); FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml")); FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml")); + FileUtils.touch(new File(base.getRoot(), "some/reports/a.xml")); + FileUtils.touch(new File(base.getRoot(), "some/reports/b.xml")); Settings settings = new Settings(); settings.setProperty(REPORT_PATH_KEY, absReportFile.toString() + ",path/**/*.xml"); - List reports = sensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); assertEquals(6, reports.size()); } + + @Test + public void testRelativeBackticksOutsideBasedirThenBackInside() throws IOException { + FileUtils.touch(new File(base.getRoot(), "path/to/supercoolreport.xml")); + FileUtils.touch(new File(base.getRoot(), "path/to/a/report.xml")); + FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/1.xml")); + FileUtils.touch(new File(base.getRoot(), "path/to/some/reports/2.xml")); + + Settings settings = new Settings(); + settings.setProperty(REPORT_PATH_KEY, "../" + base.getRoot().getName() + "/path/**/*.xml"); + + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + assertEquals(4, reports.size()); + } + + @Test + public void testRelativeExcessiveBackticks() throws IOException { + FileUtils.touch(new File(base.getRoot(), "path/to/supercoolreport.xml")); + + Settings settings = new Settings(); + // Might be valid if java.io.tmpdir is nested excessively deep -- not likely + settings.setProperty(REPORT_PATH_KEY, "../../../../../../../../../../../../../../../../../../../../../../../../" + + "../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../" + + "../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../" + + "../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../*.xml"); + + List reports = CxxReportSensor.getReports(settings, base.getRoot(), REPORT_PATH_KEY); + assertEquals(0, reports.size()); + } }