Merge pull request #30 from devcon5io/development

Development

Author: gmuecke

src/main/java/ch/devcon5/sonar/plugins/mutationanalysis/report/PitestReportParser.java

@@ -35,6 +35,7 @@
 import java.util.Collections;
 
 import ch.devcon5.sonar.plugins.mutationanalysis.model.Mutant;
+import com.ctc.wstx.exc.WstxParsingException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -132,10 +133,11 @@ public Collection<Mutant> parseMutants(final Path report) throws IOException {
     */
    Collection<Mutant> readMutants(final InputStream stream) throws XMLStreamException {
       final XMLInputFactory inf = XMLInputFactory.newInstance();
+      inf.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, Boolean.FALSE);
       final XMLStreamReader reader = inf.createXMLStreamReader(stream);
       try {
          return readMutants(reader);
-      } catch (IllegalArgumentException e){
+      } catch (IllegalArgumentException | WstxParsingException e){
          throw new XMLStreamException(e.getMessage(), reader.getLocation(),e);
       }
    }

src/test/java/ch/devcon5/sonar/plugins/mutationanalysis/report/PitestReportParserTest.java

@@ -21,17 +21,22 @@
 package ch.devcon5.sonar.plugins.mutationanalysis.report;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import javax.xml.stream.XMLStreamException;
+import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.IOException;
 import java.net.URISyntaxException;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.Collection;
 
 import ch.devcon5.sonar.plugins.mutationanalysis.model.Mutant;
 import ch.devcon5.sonar.plugins.mutationanalysis.model.MutationOperators;
+import org.apache.commons.io.IOUtils;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -198,4 +203,29 @@ public void readMutants_brokenXml_exceptionWithDetails() throws Exception {
       subject.readMutants(getClass().getResourceAsStream("PitestReportParserTest_broken.xml"));
    }
 
+
+   @Test(expected = XMLStreamException.class)
+   public void readMutants_XXE_attack_entityNotReplaced() throws Exception {
+
+      //we prepare a secret file with content that should not be disclosed
+      //this file acts as a placeholder for any file with sensitive information such as /etc/passwd
+      final String expectedSecret = "MY_SECRET";
+      File secretFile = folder.newFile("secret");
+      Files.write(secretFile.toPath(), expectedSecret.getBytes("UTF-8"));
+
+      //we forge a pitest report that should be processed by the plugin parser
+      //the attack is not hypothetical, especially in managed sonarqube instances where forged
+      //pitest reports may reveal sensitve information in the sonarqube results
+      String template = IOUtils.toString(getClass().getResourceAsStream("PitestReportParserTest_XXE.xml"));
+      String xxeAttack = template.replace("$SECRET$", secretFile.toURI().toURL().toString());
+
+      Collection<Mutant> mutants = subject.readMutants(new ByteArrayInputStream(xxeAttack.getBytes("UTF-8")));
+
+      //this code should never be  executed as the the processing of the xml should
+      //already encounter an unresolvable entity (&xxe;), causing an exception
+      Mutant mutant = mutants.iterator().next();
+      String actualSecret = mutant.getMethodDescription();
+      assertNotEquals(expectedSecret, actualSecret);
+   }
+
 }

src/test/resources/ch/devcon5/sonar/plugins/mutationanalysis/report/PitestReportParserTest_XXE.xml

@@ -0,0 +1,46 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Mutation Analysis Plugin
+  ~ Copyright (C) 2015-2018 DevCon5 GmbH, Switzerland
+  ~ info@devcon5.ch
+  ~
+  ~ This program is free software; you can redistribute it and/or
+  ~ modify it under the terms of the GNU Lesser General Public
+  ~ License as published by the Free Software Foundation; either
+  ~ version 3 of the License, or (at your option) any later version.
+  ~
+  ~ This program is distributed in the hope that it will be useful,
+  ~ but WITHOUT ANY WARRANTY; without even the implied warranty of
+  ~ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+  ~ Lesser General Public License for more details.
+  ~
+  ~ You should have received a copy of the GNU Lesser General Public License
+  ~ along with this program; if not, write to the Free Software Foundation,
+  ~ Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+  -->
+<!DOCTYPE mutations [
+        <!ELEMENT mutations ANY >
+        <!ELEMENT mutation ANY >
+        <!ATTLIST mutation detected CDATA "KILLED">
+        <!ATTLIST mutation status CDATA "true">
+        <!ELEMENT sourceFile ANY >
+        <!ELEMENT mutatedClass ANY >
+        <!ELEMENT mutatedMethod ANY >
+        <!ELEMENT methodDescription ANY >
+        <!ELEMENT lineNumber ANY >
+        <!ELEMENT mutator ANY >
+        <!ELEMENT index ANY >
+        <!ELEMENT killingTest ANY >
+        <!ENTITY xxe SYSTEM "$SECRET$" >]>
+<mutations>
+    <mutation detected='true' status='KILLED'>
+        <sourceFile>Mutant.java</sourceFile>
+        <mutatedClass>ch.devcon5.sonar.plugins.mutationanalysis.model.Mutant</mutatedClass>
+        <mutatedMethod>equals</mutatedMethod>
+        <methodDescription>&xxe;</methodDescription>
+        <lineNumber>162</lineNumber>
+        <mutator>org.pitest.mutationtest.engine.gregor.mutators.NegateConditionalsMutator</mutator>
+        <index>5</index>
+        <killingTest>ch.devcon5.sonar.plugins.mutationanalysis.model.MutantTest.testEquals_different_false(ch.devcon5.sonar.plugins.mutationanalysis.model.MutantTest)</killingTest>
+    </mutation>
+</mutations>