diff --git a/pom.xml b/pom.xml index 4aca66fc..d440ae06 100644 --- a/pom.xml +++ b/pom.xml @@ -53,7 +53,7 @@ 4.5.2 2.7 6.2.3 - 1.4.3 + 1.4.4 @@ -158,7 +158,6 @@ org.codehaus.sonar-plugins.java sonar-java-plugin - sonar-plugin ${sonar-java.version} provided diff --git a/src/main/java/org/sonar/plugins/findbugs/FindSecurityBugsJspRulesDefinition.java b/src/main/java/org/sonar/plugins/findbugs/FindSecurityBugsJspRulesDefinition.java new file mode 100644 index 00000000..487bdd81 --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/FindSecurityBugsJspRulesDefinition.java @@ -0,0 +1,46 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs; + +import org.sonar.api.server.rule.RulesDefinition; +import org.sonar.api.server.rule.RulesDefinitionXmlLoader; +import org.sonar.plugins.findbugs.language.Jsp; + +/** + * This RulesDefinition build a separate repository from the FindSecurityBugsRulesDefinition to allow a separate ruleset + * for JSP language. + * @see FindSecurityBugsRulesDefinition + */ +public class FindSecurityBugsJspRulesDefinition implements RulesDefinition { + + public static final String REPOSITORY_KEY = "findsecbugs-jsp"; + public static final String REPOSITORY_JSP_NAME = "Find Security Bugs (JSP)"; + + @Override + public void define(Context context) { + NewRepository repositoryJsp = context + .createRepository(REPOSITORY_KEY, Jsp.KEY) + .setName(REPOSITORY_JSP_NAME); + + RulesDefinitionXmlLoader ruleLoaderJsp = new RulesDefinitionXmlLoader(); + ruleLoaderJsp.load(repositoryJsp, FindSecurityBugsRulesDefinition.class.getResourceAsStream("/org/sonar/plugins/findbugs/rules-jsp.xml"), "UTF-8"); + repositoryJsp.done(); + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java index d41a035e..374dd538 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsConfiguration.java @@ -23,9 +23,12 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import org.apache.commons.io.FileUtils; +import org.apache.commons.io.FilenameUtils; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.CharEncoding; import org.apache.commons.lang.StringUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.api.BatchExtension; import org.sonar.api.CoreProperties; import org.sonar.api.PropertyType; @@ -44,11 +47,15 @@ import java.io.IOException; import java.io.InputStream; import java.io.StringWriter; -import java.util.Collection; +import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; +import java.util.Queue; public class FindbugsConfiguration implements BatchExtension { + private static final Logger LOG = LoggerFactory.getLogger(FindbugsExecutor.class); + private final FileSystem fileSystem; private final Settings settings; private final RulesProfile profile; @@ -71,18 +78,34 @@ public File getTargetXMLReport() { public edu.umd.cs.findbugs.Project getFindbugsProject() throws IOException { edu.umd.cs.findbugs.Project findbugsProject = new edu.umd.cs.findbugs.Project(); - for (File file : getSourceFiles()) { - findbugsProject.addFile(file.getCanonicalPath()); + /*for (File file : getSourceFiles()) { + if(FilenameUtils.getExtension(file.getName()).equals("java")) { + findbugsProject.addFile(file.getCanonicalPath()); + } + }*/ + + List classFilesToAnalyze = new ArrayList<>(javaResourceLocator.classFilesToAnalyze()); + + for (File file : javaResourceLocator.classpath()) { + //Will capture additional classes including precompiled JSP + if(file.isDirectory()) { // will include "/target/classes" and other non-standard folders + classFilesToAnalyze.addAll(scanForAdditionalClasses(file)); + } + + //Auxiliary dependencies + findbugsProject.addAuxClasspathEntry(file.getCanonicalPath()); } - Collection classFilesToAnalyze = javaResourceLocator.classFilesToAnalyze(); for (File classToAnalyze : classFilesToAnalyze) { findbugsProject.addFile(classToAnalyze.getCanonicalPath()); } - for (File file : javaResourceLocator.classpath()) { - findbugsProject.addAuxClasspathEntry(file.getCanonicalPath()); + if (classFilesToAnalyze.isEmpty()) { + LOG.warn("Findbugs needs sources to be compiled." + + " Please build project before executing sonar or check the location of compiled classes to" + + " make it possible for Findbugs to analyse your project."); } + copyLibs(); if (annotationsLib != null) { // Findbugs dependencies are packaged by Maven. They are not available during execution of unit tests. @@ -107,6 +130,29 @@ File saveIncludeConfigXml() throws IOException { return file; } + /** + * Scan the given folder for classes. It will catch classes from Java, JSP and more. + * + * @param folder Folder to scan + * @return + * @throws IOException + */ + public static List scanForAdditionalClasses(File folder) throws IOException { + List allFiles = new ArrayList(); + Queue dirs = new LinkedList(); + dirs.add(folder); + while (!dirs.isEmpty()) { + for (File f : dirs.poll().listFiles()) { + if (f.isDirectory()) { + dirs.add(f); + } else if (f.isFile()&& f.getName().endsWith(".class")) { + allFiles.add(f); + } + } + } + return allFiles; + } + @VisibleForTesting List getExcludesFilters() { List result = Lists.newArrayList(); diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java index 71386e81..77f70cf2 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsExecutor.java @@ -210,9 +210,7 @@ public Object call() { try { engine.execute(); return null; - } catch (InterruptedException e) { - throw Throwables.propagate(e); - } catch (IOException e) { + } catch (InterruptedException | IOException e) { throw Throwables.propagate(e); } finally { engine.dispose(); diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java index 3bcfb668..2605dec5 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsPlugin.java @@ -21,6 +21,9 @@ import com.google.common.collect.ImmutableList; import org.sonar.api.SonarPlugin; +import org.sonar.plugins.findbugs.language.Jsp; +import org.sonar.plugins.findbugs.language.JspCodeColorizerFormat; +import org.sonar.plugins.findbugs.resource.ByteCodeResourceLocator; import java.util.List; @@ -31,17 +34,22 @@ public List getExtensions() { ImmutableList.Builder extensions = ImmutableList.builder(); extensions.addAll(FindbugsConfiguration.getPropertyDefinitions()); extensions.add( + Jsp.class, + JspCodeColorizerFormat.class, FindbugsSensor.class, FindbugsConfiguration.class, FindbugsExecutor.class, - FindbugsRulesDefinition.class, FindbugsProfileExporter.class, FindbugsProfileImporter.class, FindbugsProfile.class, FindbugsSecurityAuditProfile.class, FindbugsSecurityMinimalProfile.class, + FindbugsSecurityJspProfile.class, + FindbugsRulesDefinition.class, FbContribRulesDefinition.class, - FindSecurityBugsRulesDefinition.class); + FindSecurityBugsRulesDefinition.class, + FindSecurityBugsJspRulesDefinition.class, + ByteCodeResourceLocator.class); return extensions.build(); } diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileExporter.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileExporter.java index 5bf7577d..9759b251 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileExporter.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileExporter.java @@ -25,6 +25,7 @@ import org.sonar.api.profiles.RulesProfile; import org.sonar.api.rules.ActiveRule; import org.sonar.api.utils.SonarException; +import org.sonar.plugins.findbugs.language.Jsp; import org.sonar.plugins.findbugs.xml.Bug; import org.sonar.plugins.findbugs.xml.FindBugsFilter; import org.sonar.plugins.findbugs.xml.Match; @@ -37,7 +38,7 @@ public class FindbugsProfileExporter extends ProfileExporter { public FindbugsProfileExporter() { super(/* (Godin): actually exporter key: */FindbugsRulesDefinition.REPOSITORY_KEY, FindbugsConstants.PLUGIN_NAME); - setSupportedLanguages(Java.KEY); + setSupportedLanguages(Java.KEY, Jsp.KEY); setMimeType("application/xml"); } @@ -47,7 +48,8 @@ public void exportProfile(RulesProfile profile, Writer writer) { FindBugsFilter filter = buildFindbugsFilter(Iterables.concat( profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY), profile.getActiveRulesByRepository(FbContribRulesDefinition.REPOSITORY_KEY), - profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY) + profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY), + profile.getActiveRulesByRepository(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY) )); XStream xstream = FindBugsFilter.createXStream(); writer.append("\n\n".concat(xstream.toXML(filter))); @@ -61,7 +63,8 @@ protected static FindBugsFilter buildFindbugsFilter(Iterable activeR for (ActiveRule activeRule : activeRules) { if (FindbugsRulesDefinition.REPOSITORY_KEY.equals(activeRule.getRepositoryKey()) || FbContribRulesDefinition.REPOSITORY_KEY.equals(activeRule.getRepositoryKey()) || - FindSecurityBugsRulesDefinition.REPOSITORY_KEY.equals(activeRule.getRepositoryKey())) { + FindSecurityBugsRulesDefinition.REPOSITORY_KEY.equals(activeRule.getRepositoryKey()) || + FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY.equals(activeRule.getRepositoryKey())) { Match child = new Match(); child.setBug(new Bug(activeRule.getConfigKey())); root.addMatch(child); diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileImporter.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileImporter.java index d642806f..ecc625ed 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileImporter.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsProfileImporter.java @@ -32,6 +32,7 @@ import org.sonar.api.rules.RulePriority; import org.sonar.api.rules.RuleQuery; import org.sonar.api.utils.ValidationMessages; +import org.sonar.plugins.findbugs.language.Jsp; import org.sonar.plugins.findbugs.xml.FindBugsFilter; import org.sonar.plugins.java.Java; @@ -45,7 +46,7 @@ public class FindbugsProfileImporter extends ProfileImporter { public FindbugsProfileImporter(RuleFinder ruleFinder) { super(FindbugsRulesDefinition.REPOSITORY_KEY, FindbugsConstants.PLUGIN_NAME); - setSupportedLanguages(Java.KEY); + setSupportedLanguages(Java.KEY, Jsp.KEY); this.ruleFinder = ruleFinder; } @@ -76,6 +77,9 @@ private void activateRulesByPattern(RulesProfile profile, FindBugsFilter filter, rule = ruleFinder.findByKey(FbContribRulesDefinition.REPOSITORY_KEY, patternLevel.getKey()); if (rule == null) { rule = ruleFinder.findByKey(FindSecurityBugsRulesDefinition.REPOSITORY_KEY, patternLevel.getKey()); + if (rule == null) { + rule = ruleFinder.findByKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, patternLevel.getKey()); + } } } if (rule != null) { @@ -132,8 +136,8 @@ private Iterable rules() { return Iterables.concat( ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FindbugsRulesDefinition.REPOSITORY_KEY)), ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FbContribRulesDefinition.REPOSITORY_KEY)), - ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)) - ); + ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)), + ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY))); } } diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfile.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfile.java new file mode 100644 index 00000000..29b839c5 --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfile.java @@ -0,0 +1,49 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs; + +import org.sonar.api.profiles.ProfileDefinition; +import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.utils.ValidationMessages; +import org.sonar.plugins.findbugs.language.Jsp; + +import java.io.InputStreamReader; +import java.io.Reader; + +public class FindbugsSecurityJspProfile extends ProfileDefinition { + + private static final String FINDBUGS_SECURITY_JSP_PROFILE_NAME = "FindBugs Security JSP"; + private final FindbugsProfileImporter importer; + + public FindbugsSecurityJspProfile(FindbugsProfileImporter importer) { + this.importer = importer; + } + + @Override + public RulesProfile createProfile(ValidationMessages messages) { + Reader findbugsProfile = new InputStreamReader(this.getClass().getResourceAsStream( + "/org/sonar/plugins/findbugs/profile-findbugs-security-jsp.xml")); + RulesProfile profile = importer.importProfile(findbugsProfile, messages); + profile.setLanguage(Jsp.KEY); + profile.setName(FINDBUGS_SECURITY_JSP_PROFILE_NAME); + return profile; + } + +} diff --git a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java index cbd4ad72..7435bca5 100644 --- a/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java +++ b/src/main/java/org/sonar/plugins/findbugs/FindbugsSensor.java @@ -32,9 +32,12 @@ import org.sonar.api.resources.Resource; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; +import org.sonar.plugins.findbugs.language.Jsp; +import org.sonar.plugins.findbugs.resource.ByteCodeResourceLocator; import org.sonar.plugins.java.Java; import org.sonar.plugins.java.api.JavaResourceLocator; +import java.io.File; import java.util.Collection; public class FindbugsSensor implements Sensor { @@ -45,22 +48,24 @@ public class FindbugsSensor implements Sensor { private RuleFinder ruleFinder; private FindbugsExecutor executor; private final JavaResourceLocator javaResourceLocator; + private final ByteCodeResourceLocator byteCodeResourceLocator; private final FileSystem fs; private final ResourcePerspectives perspectives; public FindbugsSensor(RulesProfile profile, RuleFinder ruleFinder, ResourcePerspectives perspectives, - FindbugsExecutor executor, JavaResourceLocator javaResourceLocator, FileSystem fs) { + FindbugsExecutor executor, JavaResourceLocator javaResourceLocator, FileSystem fs, ByteCodeResourceLocator byteCodeResourceLocator) { this.profile = profile; this.ruleFinder = ruleFinder; this.perspectives = perspectives; this.executor = executor; this.javaResourceLocator = javaResourceLocator; + this.byteCodeResourceLocator = byteCodeResourceLocator; this.fs = fs; } @Override public boolean shouldExecuteOnProject(Project project) { - return fs.hasFiles(fs.predicates().hasLanguage(Java.KEY)) + return (fs.hasFiles(fs.predicates().hasLanguage(Java.KEY)) || fs.hasFiles(fs.predicates().hasLanguage(Jsp.KEY))) && (hasActiveFindbugsRules() || hasActiveFbContribRules() || hasActiveFindSecBugsRules()); } @@ -76,14 +81,13 @@ private boolean hasActiveFindSecBugsRules() { return !profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY).isEmpty(); } + private boolean hasActiveFindSecBugsJspRules() { + return !profile.getActiveRulesByRepository(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY).isEmpty(); + } + @Override public void analyse(Project project, SensorContext context) { - if (javaResourceLocator.classFilesToAnalyze().isEmpty()) { - LOG.warn("Findbugs needs sources to be compiled." - + " Please build project before executing sonar or check the location of compiled classes to" - + " make it possible for Findbugs to analyse your project."); - return; - } + Collection collection = executor.execute(hasActiveFbContribRules(), hasActiveFindSecBugsRules()); for (ReportedBug bugInstance : collection) { @@ -92,11 +96,13 @@ public void analyse(Project project, SensorContext context) { rule = ruleFinder.findByKey(FbContribRulesDefinition.REPOSITORY_KEY, bugInstance.getType()); if (rule == null) { rule = ruleFinder.findByKey(FindSecurityBugsRulesDefinition.REPOSITORY_KEY, bugInstance.getType()); - if (rule == null) { - // ignore violations from report, if rule not activated in Sonar - LOG.warn("Findbugs rule '{}' is not active in Sonar.", bugInstance.getType()); - continue; + rule = ruleFinder.findByKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY, bugInstance.getType()); + if (rule == null) { + // ignore violations from report, if rule not activated in Sonar + LOG.warn("Findbugs rule '{}' is not active in Sonar.", bugInstance.getType()); + continue; + } } } } @@ -105,13 +111,40 @@ public void analyse(Project project, SensorContext context) { String longMessage = bugInstance.getMessage(); int line = bugInstance.getStartLine(); + //Legacy Resource resource = javaResourceLocator.findResourceByClassName(className); if (resource != null) { insertIssue(rule, resource, line, longMessage); + continue; + } + + //Regular Java class mapped to their original .java + resource = byteCodeResourceLocator.findJavaClassFile(className, project); + if (resource != null) { + insertIssue(rule, resource, line, longMessage); + continue; + } + + //Precompiled JSP mapped to their original .jsp with the correct line of code if SMAP file is present. + resource = byteCodeResourceLocator.findTemplateFile(className, project); + if (resource != null) { + if(resource.getPath().endsWith(".jsp")) { + File classFile = findOriginalClassForBug(bugInstance.getClassName()); + Integer jspLine = byteCodeResourceLocator.findJspLine(className, line, javaResourceLocator, classFile); + line = jspLine == null ? 1 : jspLine; + } + else { + line = 1; + } + insertIssue(rule, resource, line, longMessage); + continue; } + + LOG.warn("The class '"+className+"' could not be match to its original source file. It might be a dynamically generated class."); } } + private void insertIssue(Rule rule, Resource resource, int line, String message) { Issuable issuable = perspectives.as(Issuable.class, resource); if (issuable != null) { @@ -122,4 +155,26 @@ private void insertIssue(Rule rule, Resource resource, int line, String message) issuable.addIssue(builder.build()); } } + + /** + * + * @param className Class name + * @return File handle of the original class file analyzed + */ + private File findOriginalClassForBug(String className) { + String classFile = className.replaceAll("\\.","/").concat(".class"); + + for(File classPath : javaResourceLocator.classpath()) { + if(!classPath.isDirectory()) { + continue; + } + + File testClassFile = new File(classPath, classFile); + if(testClassFile.exists()) { + return testClassFile; + } + } + + return null; + } } diff --git a/src/main/java/org/sonar/plugins/findbugs/language/Jsp.java b/src/main/java/org/sonar/plugins/findbugs/language/Jsp.java new file mode 100644 index 00000000..e9289a8e --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/language/Jsp.java @@ -0,0 +1,95 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.language; + +import com.google.common.collect.Lists; +import org.apache.commons.lang.StringUtils; +import org.sonar.api.config.Settings; +import org.sonar.api.resources.AbstractLanguage; + +import java.util.List; + +/** + * This language cover JavaServer Pages (JSP). + * + * It may conflicts with the "web" language from the Web Plugin which supports multiples XML derived languages. + */ +public class Jsp extends AbstractLanguage { + + /** + * JSP key + */ + public static final String KEY = "jsp"; + + /** + * JSP language name (Acronym chosen over "JavaServer Pages" to avoid visual glitch) + */ + public static final String NAME = "JSP"; + + + /** + * Key of the file suffix parameter + */ + public static final String FILE_SUFFIXES_KEY = "sonar.jsp.file.suffixes"; + + /** + * Default Java files knows suffixes + */ + public static final String DEFAULT_FILE_SUFFIXES = ".jsp"; + + /** + * Settings of the plugin. + */ + private final Settings settings; + + /** + * Default constructor + */ + public Jsp(Settings settings) { + super(KEY, NAME); + this.settings = settings; + } + + /** + * {@inheritDoc} + * + * @see org.sonar.api.resources.AbstractLanguage#getFileSuffixes() + */ + @Override + public String[] getFileSuffixes() { + String[] suffixes = filterEmptyStrings(settings.getStringArray(FILE_SUFFIXES_KEY)); + if (suffixes.length == 0) { + suffixes = StringUtils.split(DEFAULT_FILE_SUFFIXES, ","); + } + return suffixes; + } + + private static String[] filterEmptyStrings(String[] stringArray) { + List nonEmptyStrings = Lists.newArrayList(); + for (String string : stringArray) { + if (StringUtils.isNotBlank(string.trim())) { + nonEmptyStrings.add(string.trim()); + } + } + return nonEmptyStrings.toArray(new String[nonEmptyStrings.size()]); + } + + +} diff --git a/src/main/java/org/sonar/plugins/findbugs/language/JspCodeColorizerFormat.java b/src/main/java/org/sonar/plugins/findbugs/language/JspCodeColorizerFormat.java new file mode 100644 index 00000000..1257df3c --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/language/JspCodeColorizerFormat.java @@ -0,0 +1,67 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.language; + +import org.sonar.api.web.CodeColorizerFormat; +import org.sonar.colorizer.MultilinesDocTokenizer; +import org.sonar.colorizer.RegexpTokenizer; +import org.sonar.colorizer.StringTokenizer; +import org.sonar.colorizer.Tokenizer; + +import java.util.ArrayList; +import java.util.List; + +/** + * Code Colorizer based on the Web plugin. + * + * It used the old API "CodeColorizerFormat" for simplicity. + */ +public class JspCodeColorizerFormat extends CodeColorizerFormat { + + private final List tokenizers = new ArrayList<>(); + + public JspCodeColorizerFormat() { + super(Jsp.KEY); + String tagAfter = ""; + + // == tags == + tokenizers.add(new RegexpTokenizer("", tagAfter, "?")); + tokenizers.add(new RegexpTokenizer("", tagAfter, ">")); + + // == doctype == + tokenizers.add(new RegexpTokenizer("", tagAfter, "")); + + // == comments == + tokenizers.add(new MultilinesDocTokenizer("", "", tagAfter)); + tokenizers.add(new MultilinesDocTokenizer("<%--", "--%>", "", tagAfter)); + + // == expressions == + tokenizers.add(new MultilinesDocTokenizer("<%@", "%>", "", tagAfter)); + tokenizers.add(new MultilinesDocTokenizer("<%", "%>", "", tagAfter)); + + // == tag properties == + tokenizers.add(new StringTokenizer("", tagAfter)); + } + + @Override + public List getTokenizers() { + return tokenizers; + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java b/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java new file mode 100644 index 00000000..a308620b --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/resource/ByteCodeResourceLocator.java @@ -0,0 +1,165 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.resource; + +import org.apache.commons.io.IOUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.sonar.api.BatchExtension; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.plugins.java.api.JavaResourceLocator; + +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +/** + * Utility method related to mapped class name to various resources and extracting addition information. + */ +public class ByteCodeResourceLocator implements BatchExtension { + + + private static final Logger LOG = LoggerFactory.getLogger(ByteCodeResourceLocator.class); + + /** + * Find the file system location of a given class name.
+ * (ie : test.SomeClass -> src/main/java/test/SomeClass.java) + * + * @param className Class name to look for + * @param project Sonar roject information which contains source locations + * @return Java source file that conrespond to the class name specified. + */ + public Resource findJavaClassFile(String className, Project project) { + for(File sourceDir : project.getFileSystem().getSourceDirs()) { + File potentialFile = new File(sourceDir, className.replaceAll("\\.","/")+".java"); + if(potentialFile.exists()) { + org.sonar.api.resources.File file = org.sonar.api.resources.File.fromIOFile(potentialFile, project); + return file; + } + } + return null; + } + + /** + * JSP files are compile to class with pseudo packages and class name that vary based on the compiler used. + * Multiples patterns are test against the available sources files.
+ * (ie : test.index_jsp -> src/main/webapp/test/index.jsp) + *
+ * Their is a certain level of guessing since their could always be a class following the same pattern of colliding + * precompiled jsp. (same pseudo package, same class format, etc.) + * + * @param className Class name of the precompiled jsp + * @param project Sonar roject information which contains source locations + * @return The + */ + public Resource findTemplateFile(String className, Project project) { + List potentialJspFilenames = new ArrayList<>(); + + + //Weblogic APPC precompiled form + //Expected class name: "jsp_servlet._folder1._folder2.__helloworld" + if(className.startsWith("jsp_servlet")) { + String jspFile = className.substring(11).replaceFirst("\\.__([^\\.]+)$", "/$1\\.jsp").replace("._", "/"); + potentialJspFilenames.add(jspFile); + } + //Jetty and Tomcat JSP precompiled form + //Expected class name: "jsp.folder1.folder2.hello_005fworld_jsp" + if (className.endsWith("_jsp")) { + String jspFileFromClass = className.replaceAll("\\.", "/").replaceAll("_005f", "_").replaceAll("_jsp", ".jsp"); + + potentialJspFilenames.add(jspFileFromClass); + + for(String packageName : Arrays.asList("jsp/", "org/apache/jsp/")) { + if(jspFileFromClass.startsWith(packageName)) + potentialJspFilenames.add(jspFileFromClass.substring(packageName.length())); + } + } + + //Source directories will include typically : /src/main/java and /src/main/webapp + for(File sourceDir : project.getFileSystem().getSourceDirs()) { + for(String jspFilename : potentialJspFilenames) { + + File jspFile = new File(sourceDir, jspFilename); + if(jspFile.exists()) { + org.sonar.api.resources.File file = org.sonar.api.resources.File.fromIOFile(jspFile, project); + return file; + } + } + } + return null; + } + + /** + * Map Java line number to JSP line number based on SMAP + * + * The smap can be either embedded in the class file or alternatively place in separate file. + * + * @param className Class name + * @param originalLine Line of code of the auto-generated Java line (.jsp -> .java -> .class) + * @param javaResourceLocator Locator that can enumerate folders containing classes + * @param classFile (Optional) + * @return JSP line number + */ + public Integer findJspLine(String className, int originalLine, JavaResourceLocator javaResourceLocator, + File classFile) { + //Extract the SMAP (JSR45) from the class file (SourceDebugExtension section) + try (InputStream in = new FileInputStream(classFile)) { + DebugExtensionExtractor debug = new DebugExtensionExtractor(); + return getJspLineNumberFromSmap(debug.getDebugExtFromClass(in), originalLine); + } + catch (IOException e) { + LOG.warn("An error occurs while opening classfile : " + classFile.getPath()); + } + LOG.debug("No smap file found for the class: " + className); + + //Extract the SMAP (JSR45) from the separated smap file + File smapFile = new File(classFile.getPath()+".smap"); + if(smapFile.exists()) { + try (InputStream smapInputStream = new FileInputStream(smapFile)) { + return getJspLineNumberFromSmap(IOUtils.toString(smapInputStream), originalLine); + } + catch (IOException e) { + LOG.debug("Unable to open smap file : " + smapFile.getAbsolutePath()); + throw new RuntimeException(e); + } + } + + LOG.debug("No smap mapping found."); + return null; //No smap file is present. + } + + /** + * + * @param smap SMAP content (See smap.txt sample in test directories) + * @param originalLine Java source code line number + * @return JSP line number + * @throws IOException + */ + private int getJspLineNumberFromSmap(String smap, Integer originalLine) throws IOException { + SmapParser parser = new SmapParser(smap); + int[] mapping = parser.getScriptLineNumber(originalLine); + return mapping[1]; + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractor.java b/src/main/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractor.java new file mode 100644 index 00000000..78982f3a --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractor.java @@ -0,0 +1,63 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.resource; + +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.ClassVisitor; +import org.objectweb.asm.Opcodes; + +import java.io.IOException; +import java.io.InputStream; + +/** + * Load SourceDebugExtension section from class file. + * + * SourceDebugExtension is use to store optionally the line of code mapping. + * The code content if not empty is must likely format according to JSR45 (SMAP). + */ +public class DebugExtensionExtractor { + + public String getDebugExtFromClass(InputStream classIn) throws IOException { + + AbstractClassVisitor visitor = new AbstractClassVisitor(); + ClassReader classReader= new ClassReader(classIn); + classReader.accept(visitor, 0); + + return visitor.debug; + } + + + private class AbstractClassVisitor extends ClassVisitor { + + protected String source; + protected String debug; + + public AbstractClassVisitor() { + super(Opcodes.ASM5); + } + + @Override + public void visitSource(String source, String debug) { + super.visitSource(source, debug); + this.source = source; + this.debug = debug; + } + } +} diff --git a/src/main/java/org/sonar/plugins/findbugs/resource/SmapParser.java b/src/main/java/org/sonar/plugins/findbugs/resource/SmapParser.java new file mode 100644 index 00000000..a4ee246c --- /dev/null +++ b/src/main/java/org/sonar/plugins/findbugs/resource/SmapParser.java @@ -0,0 +1,176 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.resource; + +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStreamReader; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * This class is a highly modified version of Michael Schierl's "SmapParser.java". + * It was test with Jetty and WebLogic SMAP. + * + * ======= + * + * SmapParser.java - Parse source debug extensions and + * enhance stack traces. + * + * Copyright (c) 2012 Michael Schierl + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * - Redistributions of source code must retain the above copyright notice, + * this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * - Neither name of the copyright holders nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND THE CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * HOLDERS OR THE CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, + * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS + * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR + * TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE + * USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * + * Utility class to parse Source Debug Extensions and enhance stack traces. + * + * Note that only the first stratum is parsed and used. + * + * @author Michael Schierl + */ +public class SmapParser { + + private String javaFilename; + private final Map fileinfo = new HashMap<>(); + private final Map java2jsp = new HashMap<>(); + + private static final Pattern LINE_INFO_PATTERN = Pattern.compile("([0-9]+)(?:#([0-9]+))?(?:,([0-9]+))?:([0-9]+)(?:,([0-9]+))?"); + + private static String getLine(BufferedReader reader) throws IOException { + String s = reader.readLine(); + //System.out.println("))"+s); + if (s == null) { + throw new IOException("EOF parsing SMAP"); + } + return s; + } + + public SmapParser(String smap) throws IOException { + //BufferedReader is use to support multiple types of line return + BufferedReader reader = new BufferedReader(new InputStreamReader(new ByteArrayInputStream(smap.getBytes()))); + + String header = getLine(reader); //SMAP + javaFilename = getLine(reader); //*****.java + String jsp = getLine(reader); //JSP or alternative script + String stratum = getLine(reader); //*S JSP + String f = getLine(reader); //*F + + if (!header.equals("SMAP") || !stratum.startsWith("*S ") || !f.equals("*F")) + throw new IllegalArgumentException("Unexpected SMAP file format"); + + //Parse the file info section (*F) + String line; + while((line = getLine(reader)) != null && !line.equals("*L")) { + String path = null; + if (line.startsWith("+ ")) { + path = getLine(reader); + line = line.substring(2); + } + + int pos = line.indexOf(" "); + int fileNum = Integer.parseInt(line.substring(0, pos)); + String name = line.substring(pos + 1); + fileinfo.put(fileNum, new FileInfo(name, path == null ? name : path)); + } + + //Parse the line number mapping section (*L) + int lastLFI = 0; + while((line = getLine(reader)) != null && !line.equals("*E")) { + + if (!line.startsWith("*")) { + + Matcher m = LINE_INFO_PATTERN.matcher(line); + if (!m.matches()) + throw new IllegalArgumentException(line); + + int inputStartLine = Integer.parseInt(m.group(1)); + int lineFileID = m.group(2) == null ? lastLFI : Integer.parseInt(m.group(2)); + int repeatCount = m.group(3) == null ? 1 : Integer.parseInt(m.group(3)); + int outputStartLine = Integer.parseInt(m.group(4)); + int outputLineIncrement = m.group(5) == null ? 1 : Integer.parseInt(m.group(5)); + + for (int i = 0; i < repeatCount; i++) { + int[] inputMapping = new int[]{ lineFileID, inputStartLine + i}; + int baseOL = outputStartLine + i * outputLineIncrement; + for (int ol = baseOL; ol < baseOL + outputLineIncrement; ol++) { + if (!java2jsp.containsKey(ol)) + java2jsp.put(ol, inputMapping); + } + } + lastLFI = lineFileID; + } + } + } + + public String getJavaFilename() { + return javaFilename; + } + + public String getScriptFilename() { + FileInfo f = fileinfo.get(0); + return f.name; + } + + public int[] getScriptLineNumber(Integer lineNo) { + return java2jsp.get(lineNo); + } + + private static class FileInfo { + public final String name; + public final String path; + + public FileInfo(String name, String path) { + this.name = name; + this.path = path; + } + } +} \ No newline at end of file diff --git a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-audit.xml b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-audit.xml index 37e6fc97..3cb0b076 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-audit.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-audit.xml @@ -152,9 +152,6 @@ - - - @@ -203,9 +200,6 @@ - - - diff --git a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-jsp.xml b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-jsp.xml new file mode 100644 index 00000000..2fead297 --- /dev/null +++ b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-jsp.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-minimal.xml b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-minimal.xml index a136208c..aeb366e3 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-minimal.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs-security-minimal.xml @@ -95,9 +95,6 @@ - - - @@ -131,9 +128,6 @@ - - - diff --git a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs.xml b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs.xml index eb2c2b51..bbe40c74 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/profile-findbugs.xml @@ -55,9 +55,6 @@ - - - diff --git a/src/main/resources/org/sonar/plugins/findbugs/rules-findsecbugs.xml b/src/main/resources/org/sonar/plugins/findbugs/rules-findsecbugs.xml index 0c0c2b70..d24b8730 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/rules-findsecbugs.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/rules-findsecbugs.xml @@ -1,6 +1,6 @@ - Security - Predictable Pseudo Random Number Generator + Security - Predictable pseudorandom number generator PREDICTABLE_RANDOM <p>The use of a predictable random value can lead to vulnerabilities when used in certain security critical contexts. For example, when the value is used as:</p> <ul> @@ -42,7 +42,7 @@ String generateSecretToken() { security - Security - Untrusted Servlet Parameter + Security - Untrusted servlet parameter SERVLET_PARAMETER <p>The Servlet can read GET and POST parameters from various methods. The value obtained should be considered unsafe. You may need to validate or sanitize those values before passing them to sensitive APIs such as:</p> @@ -65,7 +65,7 @@ You may need to validate or sanitize those values before passing them to sensiti security - Security - Untrusted Content-Type Header + Security - Untrusted Content-Type header SERVLET_CONTENT_TYPE <p> The HTTP header Content-Type can be controlled by the client. As such, its value should not be used in any security critical decisions. @@ -79,10 +79,10 @@ The HTTP header Content-Type can be controlled by the client. As such, its value security - Security - Untrusted Hostname Header + Security - Untrusted Hostname header SERVLET_SERVER_NAME - <p>The hostname header can be controlled by the client. As such, its value should not be used in any security critical decisions. -Both <code>ServletRequest.getServerName()</code> and <code>HttpServletRequest.getHeader("Host")</code> have the same + <p>The hostname header can be controlled by the client. As such, its value should not be used in any security critical decisions. +Both <code>ServletRequest.getServerName()</code> and <code>HttpServletRequest.getHeader("Host")</code> have the same behavior which is to extract the <code>Host</code> header.</p> <pre> GET /testpage HTTP/1.1 @@ -102,14 +102,14 @@ decisions you make with respect to a request. security - Security - Untrusted Session Cookie Value + Security - Untrusted session cookie value SERVLET_SESSION_ID <p> The method <a href="http://docs.oracle.com/javaee/6/api/javax/servlet/http/HttpServletRequest.html#getRequestedSessionId()"><code>HttpServletRequest.getRequestedSessionId()</code></a> typically returns the value of the cookie <code>JSESSIONID</code>. This value is normally only accessed by the session management logic and not normal developer code. </p> <p> -The value passed to the client is generally an alphanumeric value (e.g., <code>JSESSIONID=jp6q31lq2myn</code>). However, the value can be altered by the client. +The value passed to the client is generally an alphanumeric value (e.g., <code>JSESSIONID=jp6q31lq2myn</code>). However, the value can be altered by the client. The following HTTP request illustrates the potential modification. <pre> GET /somePage HTTP/1.1 @@ -118,8 +118,8 @@ User-Agent: Mozilla/5.0 Cookie: JSESSIONID=Any value of the user&#39;s choice!!??'''&quot;&gt; </pre> </p> -<p>As such, the JSESSIONID should only be used to see if its value matches an existing session ID. If it does not, the user should be -considered an unauthenticated user. In addition, the session ID value should never be logged. If it is, then the log file could contain +<p>As such, the JSESSIONID should only be used to see if its value matches an existing session ID. If it does not, the user should be +considered an unauthenticated user. In addition, the session ID value should never be logged. If it is, then the log file could contain valid active session IDs, allowing an insider to hijack any sessions whose IDs have been logged and are still active. </p> <br/> @@ -133,14 +133,14 @@ valid active session IDs, allowing an insider to hijack any sessions whose IDs h security - Security - Untrusted Query String + Security - Untrusted query string SERVLET_QUERY_STRING <p>The query string is the concatenation of the GET parameter names and values. Parameters other than those intended can be passed in.</p> <p>For the URL request <code>/app/servlet.htm?a=1&b=2</code>, the query string extract will be <code>a=1&b=2</code></p> -<p>Just as is true for individual parameter values retrieved via methods like <code>HttpServletRequest.getParameter()</code>, +<p>Just as is true for individual parameter values retrieved via methods like <code>HttpServletRequest.getParameter()</code>, the value obtained from <code>HttpServletRequest.getQueryString()</code> should be considered unsafe. -You may need to validate or sanitize anything pulled from the query string before passing it to sensitive APIs. +You may need to validate or sanitize anything pulled from the query string before passing it to sensitive APIs. </p> <br/> <p> @@ -151,10 +151,10 @@ You may need to validate or sanitize anything pulled from the query string befor security - Security - HTTP Headers Untrusted + Security - HTTP headers untrusted SERVLET_HEADER - <p>Request headers can easily be altered by the requesting user. In general, no assumption should be made that -the request came from a regular browser without modification by an attacker. As such, it is recommended that you + <p>Request headers can easily be altered by the requesting user. In general, no assumption should be made that +the request came from a regular browser without modification by an attacker. As such, it is recommended that you not trust this value in any security decisions you make with respect to a request.</p> <br/> <p> @@ -165,7 +165,7 @@ not trust this value in any security decisions you make with respect to a reques security - Security - Untrusted Referer Header + Security - Untrusted Referer header SERVLET_HEADER_REFERER <p> Behavior: @@ -190,7 +190,7 @@ Recommendations: security - Security - Untrusted User-Agent Header + Security - Untrusted User-Agent header SERVLET_HEADER_USER_AGENT <p>The header "User-Agent" can easily be spoofed by the client. Adopting different behaviors based on the User-Agent (for crawler UA) is not recommended.</p> @@ -203,7 +203,7 @@ crawler UA) is not recommended.</p> security - Security - Potentially Sensitive Data in Cookie + Security - Potentially sensitive data in a cookie COOKIE_USAGE <p>The information stored in a custom cookie should not be sensitive or related to the session. In most cases, sensitive data should only be stored in session and referenced by the user's session cookie. See HttpSession (HttpServletRequest.getSession())</p> @@ -217,9 +217,9 @@ and referenced by the user's session cookie. See HttpSession (HttpServletRequest security - Security - Potential Path Traversal (File Read) + Security - Potential Path Traversal (file read) PATH_TRAVERSAL_IN - <p>A file is opened to read its content. The filename comes from an <b>input</b> parameter. + <p>A file is opened to read its content. The filename comes from an <b>input</b> parameter. If an unfiltered parameter is passed to this file API, files from an arbitrary filesystem location could be read.</p> <p>This rule identifies <b>potential</b> path traversal vulnerabilities. In many cases, the constructed file path cannot be controlled by the user. If that is the case, the reported instance is a false positive.</p> @@ -237,9 +237,9 @@ by the user. If that is the case, the reported instance is a false positive.< security - Security - Potential Path Traversal (File Write) + Security - Potential Path Traversal (file write) PATH_TRAVERSAL_OUT - <p>A file is opened to write to its contents. The filename comes from an <b>input</b> parameter. + <p>A file is opened to write to its contents. The filename comes from an <b>input</b> parameter. If an unfiltered parameter is passed to this file API, files at an arbitrary filesystem location could be modified.</p> <p>This rule identifies <b>potential</b> path traversal vulnerabilities. In many cases, the constructed file path cannot be controlled by the user. If that is the case, the reported instance is a false positive.</p> @@ -273,16 +273,16 @@ by the user. If that is the case, the reported instance is a false positive.< security - Security - FilenameUtils Not Filtering Null Bytes + Security - FilenameUtils not filtering null bytes WEAK_FILENAMEUTILS <p>Some FilenameUtils' methods don't filter NULL bytes (<code>0x00</code>).</p> <p>If a null byte is injected into a filename, if this filename is passed to the underlying OS, the file retrieved will be the name of the file that is specified prior to the NULL byte, since at the OS level, all strings are terminated by a null byte even -though Java itself doesn't care about null bytes or treat them special. This OS behavior can be used to bypass filename validation +though Java itself doesn't care about null bytes or treat them special. This OS behavior can be used to bypass filename validation that looks at the end of the filename (e.g., endswith ".log") to make sure its a safe file to access.</p> <p>To fix this, two things are recommended: <ul> -<li>Upgrade to Java 7 update 40 or later, or Java 8+ since +<li>Upgrade to Java 7 update 40 or later, or Java 8+ since <a href="http://bugs.java.com/bugdatabase/view_bug.do?bug_id=8014846">NULL byte injection in filenames is fixed in those versions</a>.</li> <li>Strongly validate any filenames provided by untrusted users to make sure they are valid (i.e., don't contain null, don't include path characters, etc.)</li> </ul> @@ -301,7 +301,7 @@ that looks at the end of the filename (e.g., endswith ".log") to make sure its a security - Security - TrustManager Implementation Empty + Security - TrustManager implementation empty WEAK_TRUST_MANAGER <p>Empty TrustManager implementations are often used to connect easily to a host that is not signed by a root <a href="http://en.wikipedia.org/wiki/Certificate_authority">certificate authority</a>. As a consequence, this is vulnerable to @@ -311,7 +311,7 @@ since the client will trust any certificate. <p> A TrustManager allowing specific certificates (based on a truststore for example) should be built. Detailed information for a proper implementation is at: -<a href="http://stackoverflow.com/a/6378872/89769">[1]</a> +<a href="http://stackoverflow.com/a/6378872/89769">[1]</a> <a href="http://stackoverflow.com/a/5493452/89769">[2]</a> </p> <br/> @@ -327,7 +327,7 @@ Detailed information for a proper implementation is at: security - Security - Found JAX-WS SOAP Endpoint + Security - Found JAX-WS SOAP endpoint JAXWS_ENDPOINT <p>This method is part of a SOAP Web Service (JSR224).</p> <p> @@ -349,7 +349,7 @@ Detailed information for a proper implementation is at: security - Security - Found JAX-RS REST Endpoint + Security - Found JAX-RS REST endpoint JAXRS_ENDPOINT <p>This method is part of a REST Web Service (JSR311).</p> <p> @@ -376,10 +376,10 @@ Detailed information for a proper implementation is at: security - Security - Found Tapestry Page + Security - Found Tapestry page TAPESTRY_ENDPOINT - <p>A Tapestry endpoint was discovered at application startup. Tapestry apps are structured with a backing Java class and a corresponding -Tapestry Markup Language page (a .tml file) for each page. When a request is received, the GET/POST parameters are mapped to specific + <p>A Tapestry endpoint was discovered at application startup. Tapestry apps are structured with a backing Java class and a corresponding +Tapestry Markup Language page (a .tml file) for each page. When a request is received, the GET/POST parameters are mapped to specific inputs in the backing Java class. The mapping is either done with fieldName:</p> <pre><code> [...] @@ -398,7 +398,7 @@ inputs in the backing Java class. The mapping is either done with fieldName:< [...] </code></pre> <p>The page is mapped to the view <code>[/resources/package/PageName].tml.</code></p> -<p>Each Tapestry page in this application should be researched to make sure all inputs that are automatically +<p>Each Tapestry page in this application should be researched to make sure all inputs that are automatically mapped in this way are properly validated before they are used.</p> <br/> <p> @@ -410,11 +410,11 @@ mapped in this way are properly validated before they are used.</p> security - Security - Found Wicket Page + Security - Found Wicket WebPage WICKET_ENDPOINT - <p>This class represents a Wicket WebPage. Input is automatically read from a PageParameters instance passed to the constructor. + <p>This class represents a Wicket WebPage. Input is automatically read from a PageParameters instance passed to the constructor. The current page is mapped to the view [/package/WebPageName].html.</p> -<p>Each Wicket page in this application should be researched to make sure all inputs that are automatically +<p>Each Wicket page in this application should be researched to make sure all inputs that are automatically mapped in this way are properly validated before they are used.</p> <br/> <p> @@ -426,7 +426,7 @@ mapped in this way are properly validated before they are used.</p> security - Security - MessageDigest Is Weak + Security - Message digest is weak WEAK_MESSAGE_DIGEST <p>The algorithm used is not a recommended MessageDigest.</p> <p><a href="http://csrc.nist.gov/groups/ST/toolkit/secure_hashing.html">NIST</a> recommends the use of SHA-1, SHA-224*, SHA-256, SHA-384, SHA-512, SHA-512/224, or SHA-512/256.</p> @@ -444,7 +444,7 @@ mapped in this way are properly validated before they are used.</p> security - Security - MessageDigest Is Custom + Security - Message digest is custom CUSTOM_MESSAGE_DIGEST <p>Implementing a custom MessageDigest is error-prone.</p> <p><a href="http://csrc.nist.gov/groups/ST/toolkit/secure_hashing.html">NIST</a> recommends the use of SHA-1, SHA-224*, SHA-256, SHA-384, SHA-512, SHA-512/224, or SHA-512/256.</p> @@ -479,7 +479,7 @@ sha256Digest.update(password.getBytes());</pre> security - Security - Tainted Filename Read + Security - Tainted filename read FILE_UPLOAD_FILENAME <p>The filename provided by the FileUpload API can be tampered with by the client to reference unauthorized files.</p> <p>For example:</p> @@ -487,8 +487,8 @@ sha256Digest.update(password.getBytes());</pre> <li><code>"../../../config/overide_file"</code></li> <li><code>"shell.jsp\u0000expected.gif"</code></li> </ul> -<p>Therefore, such values should not be passed directly to the filesystem API. If acceptable, the application should generate its -own file names and use those. Otherwise, the provided filename should be properly validated to ensure it's properly structured, +<p>Therefore, such values should not be passed directly to the filesystem API. If acceptable, the application should generate its +own file names and use those. Otherwise, the provided filename should be properly validated to ensure it's properly structured, contains no unauthorized path characters (e.g., / \), and refers to an authorized file.</p> <br/> <p> @@ -509,17 +509,17 @@ contains no unauthorized path characters (e.g., / \), and refers to an authorize Security - Regex DOS (ReDOS) REDOS <p> - Regular expressions (regexs) are frequently subject to Denial of Service (DOS) attacks (called ReDOS). This is due to the fact that regex engines + Regular expressions (regexs) are frequently subject to Denial of Service (DOS) attacks (called ReDOS). This is due to the fact that regex engines may take a large amount of time when analyzing certain strings, depending on how the regex is defined. <p> - For example, for the regex: <b>^(a+)+$</b>, the input "<code>aaaaaaaaaaaaaaaaX</code>" will cause the regex engine to analyze 65536 + For example, for the regex: <b>^(a+)+$</b>, the input "<code>aaaaaaaaaaaaaaaaX</code>" will cause the regex engine to analyze 65536 different paths.<sup>[1] Example taken from OWASP references</sup></p> <p> -Therefore, it is possible that a single request may cause a large amount of computation on the server side. -The problem with this regex, and others like it, is that there are two different ways the same input character can be accepted by the -Regex due to the + (or a *) inside the parenthesis, and the + (or a *) outside the parenthesis. The way this is written, either + could -consume the character 'a'. To fix this, the regex should be rewritten to eliminate the ambiguity. For example, this could simply be -rewritten as: <b>^a+$</b>, which is presumably what the author meant anyway (any number of a's). Assuming that's what the original +Therefore, it is possible that a single request may cause a large amount of computation on the server side. +The problem with this regex, and others like it, is that there are two different ways the same input character can be accepted by the +Regex due to the + (or a *) inside the parenthesis, and the + (or a *) outside the parenthesis. The way this is written, either + could +consume the character 'a'. To fix this, the regex should be rewritten to eliminate the ambiguity. For example, this could simply be +rewritten as: <b>^a+$</b>, which is presumably what the author meant anyway (any number of a's). Assuming that's what the original regex meant, this new regex can be evaluated quickly, and is not subject to ReDOS. </p> <br/> @@ -533,7 +533,7 @@ regex meant, this new regex can be evaluated quickly, and is not subject to ReDO security - Security - XML Parsing Vulnerable to XXE (SAXParser) + Security - XML parsing vulnerable to XXE (SAXParser) XXE_SAXPARSER <!--XXE_GENERIC_START--> <h3>Attack</h3> @@ -613,7 +613,7 @@ parser.parse(inputStream, customHandler);</pre> security - Security - XML Parsing Vulnerable to XXE (XMLReader) + Security - XML parsing vulnerable to XXE (XMLReader) XXE_XMLREADER <!--XXE_GENERIC_START--> <h3>Attack</h3> @@ -693,7 +693,7 @@ reader.parse(new InputSource(inputStream));</pre> security - Security - XML Parsing Vulnerable to XXE (DocumentBuilder) + Security - XML parsing vulnerable to XXE (DocumentBuilder) XXE_DOCUMENT <!--XXE_GENERIC_START--> <h3>Attack</h3> @@ -796,25 +796,25 @@ could be exposed. This could allow an attacker to access unauthorized data or ma security - Security - Found Struts 1 Action + Security - Found Struts 1 endpoint STRUTS1_ENDPOINT <p>This class is a Struts 1 Action.</p> -<p>Once a request is routed to this controller, a Form object will automatically be instantiated that contains the HTTP parameters. +<p>Once a request is routed to this controller, a Form object will automatically be instantiated that contains the HTTP parameters. The use of these parameters should be reviewed to make sure they are used safely.</p> security - Security - Found Struts 2 Endpoint + Security - Found Struts 2 endpoint STRUTS2_ENDPOINT <p>In Struts 2, the endpoints are Plain Old Java Objects (POJOs) which means no Interface/Class needs to be implemented/extended.</p> <p>When a request is routed to its controller (like the selected class), the supplied HTTP parameters are automatically mapped to setters for -the class. Therefore, all setters of this class should be considered as untrusted input even if the form doesn't include those values. +the class. Therefore, all setters of this class should be considered as untrusted input even if the form doesn't include those values. An attacker can simply provide additional values in the request, and they will be set in the object anyway, as long as that object has such a setter. The use of these parameters should be reviewed to make sure they are used safely.</p> security - Security - Found Spring Endpoint + Security - Found Spring endpoint SPRING_ENDPOINT <p>This class is a Spring Controller. All methods annotated with <code>RequestMapping</code> are reachable remotely. This class should be analyzed to make sure that remotely exposed methods are safe to expose to potential attackers.</p> @@ -1152,10 +1152,99 @@ public void parseExpressionInterface(Person personObj,String property) { cwe security + + Security - Potential HTTP Response Splitting + HTTP_RESPONSE_SPLITTING + <p> + When an HTTP request contains unexpected CR and LF characters, the server may respond with an output stream + that is interpreted as two different HTTP responses (instead of one). + An attacker can control the second response and mount attacks such as cross-site scripting and cache poisoning attacks. + According to OWASP, the issue has been fixed in virtually all modern Java EE application servers, but it is still better to validate the input. + If you are concerned about this risk, you should test on the platform of concern to see + if the underlying platform allows for CR or LF characters to be injected into headers. + This weakness is reported with lower priority than SQL injection etc., + if you are using a vulnerable platform, please check low-priority warnings too. +</p> +<br/> +<p> +<b>Code at risk:</b><br/> +<pre>String author = request.getParameter(AUTHOR_PARAMETER); +// ... +Cookie cookie = new Cookie("author", author); +response.addCookie(cookie);</pre> +</p> +<br/> +<p> + <b>References</b><br/> + <a href="https://www.owasp.org/index.php/HTTP_Response_Splitting">OWASP: HTTP Response Splitting</a><br/> + <a href="http://cwe.mitre.org/data/definitions/113.html">CWE-113: Improper Neutralization of CRLF Sequences in HTTP Headers ('HTTP Response Splitting')</a> + <a href="http://cwe.mitre.org/data/definitions/93.html">CWE-93: Improper Neutralization of CRLF Sequences ('CRLF Injection')</a><br/> +</p> + owasp-a1 + injection + cwe + security + + + Security - Potential CRLF Injection for logs + CRLF_INJECTION_LOGS + <p> + When data from untusted source are put into a logger and not neutralized correctly, + an attacker could forge log entries or include malicious content. + Inserted false entries could be used to skew statistics, distract the administrator + or even to implicate another party in the commission of a malicious act. + If the log file is processed automatically, the attacker can render the file unusable + by corrupting the format of the file or injecting unexpected characters. + An attacker may also inject code or other commands into the log file and take advantage + of a vulnerability in the log processing utility (e.g. command injection or XSS). +</p> +<br/> +<p> +<b>Code at risk:</b><br/> +<pre>String val = request.getParameter("val"); +try { + number = Integer.parseInt(val); +} catch (NumberFormatException) { + log.info("Failed to parse val = " + val); +}</pre> +</p> +<br/> +<p> + <b>References</b><br/> + <a href="http://cwe.mitre.org/data/definitions/117.html">CWE-117: Improper Output Neutralization for Logs</a><br/> + <a href="http://cwe.mitre.org/data/definitions/93.html">CWE-93: Improper Neutralization of CRLF Sequences ('CRLF Injection')</a><br/> +</p> + owasp-a1 + injection + cwe + security + + + Security - Potential external control of configuration + EXTERNAL_CONFIG_CONTROL + <p> + Allowing external control of system settings can disrupt service or cause an application + to behave in unexpected, and potentially malicious ways. + An attacker could cause an error by providing a nonexistent catalog name + or connect to an unauthorized portion of the database. +</p> +<br/> +<p> +<b>Code at risk:</b><br/> +<pre>conn.setCatalog(request.getParameter("catalog"));</pre> +</p> +<br/> +<p> + <b>References</b><br/> + <a href="http://cwe.mitre.org/data/definitions/15.html">CWE-15: External Control of System or Configuration Setting</a><br/> +</p> + cwe + security + Security - Bad hexadecimal concatenation BAD_HEXA_CONVERSION - <p>When converting a byte array containing a hash signature to a human readable string, a conversion mistake can be made if + <p>When converting a byte array containing a hash signature to a human readable string, a conversion mistake can be made if the array is read byte by byte. The following sample illustrates the use of Integer.toHexString() which will trim any leading zeroes from each byte of the computed hash value. <pre> @@ -1170,7 +1259,7 @@ for(byte b :resultBytes) { return stringBuilder.toString();</pre> </p> <p> -This mistake weakens the hash value computed since it introduces more collisions. +This mistake weakens the hash value computed since it introduces more collisions. For example, the hash values "0x0679" and "0x6709" would both output as "679" for the above function. </p> @@ -1188,7 +1277,7 @@ stringBuilder.append( String.format( "%02X", b ) );</pre> security - Security - Hazelcast Symmetric Encryption + Security - Hazelcast symmetric encryption HAZELCAST_SYMMETRIC_ENCRYPTION <p>The network communications for Hazelcast is configured to use a symmetric cipher (probably DES or blowfish).</p> <p>Those ciphers alone do not provide integrity or secure authentication. The use of asymmetric encryption is preferred.</p> @@ -1206,10 +1295,10 @@ stringBuilder.append( String.format( "%02X", b ) );</pre> security - Security - NullCipher Unsafe + Security - NullCipher is insecure NULL_CIPHER <p> -The NullCipher is rarely used intentionally in production applications. It implements the Cipher interface by returning ciphertext +The NullCipher is rarely used intentionally in production applications. It implements the Cipher interface by returning ciphertext identical to the supplied plaintext. In a few contexts, such as testing, a NullCipher may be appropriate. </p> <p> @@ -1269,10 +1358,10 @@ to do this correctly. security - Security - DES / DESede Unsafe + Security - DES/DESede is insecure DES_USAGE <p> -DES and DESede (3DES) are not considered strong ciphers for modern applications. Currently, NIST recommends the +DES and DESede (3DES) are not considered strong ciphers for modern applications. Currently, NIST recommends the usage of AES block ciphers instead of DES/3DES. </p> <p> @@ -1299,7 +1388,7 @@ byte[] cipherText = c.doFinal(plainText);</pre> security - Security - RSA NoPadding Unsafe + Security - RSA with no padding is insecure RSA_NO_PADDING <p> The software uses the RSA algorithm but does not incorporate Optimal Asymmetric Encryption Padding (OAEP), which might weaken the encryption. @@ -1365,7 +1454,7 @@ certainly shared in open source. To be managed safely, passwords and secret keys SecretKeySpec spec = new SecretKeySpec(key, "AES"); Cipher aes = Cipher.getInstance("AES"); aes.init(Cipher.ENCRYPT_MODE, spec); -return aesCipher.doFinal(secretData);</pre> +return aesCipher.doFinal(secretData);</pre> </p> <br/> <p> @@ -1376,7 +1465,7 @@ return aesCipher.doFinal(secretData);</pre> security - Security - Struts Form Without Input Validation + Security - Struts Form without input validation STRUTS_FORM_VALIDATION <p> Form inputs should have minimal input validation. Preventive validation helps provide defense in depth against a variety of risks. @@ -1407,7 +1496,7 @@ public class RegistrationForm extends ValidatorForm { security - Security - XSSRequestWrapper is Weak XSS Protection + Security - XSSRequestWrapper is a weak XSS protection XSS_REQUEST_WRAPPER <p> An implementation of <code>HttpServletRequestWrapper</code> called <code>XSSRequestWrapper</code> was published through @@ -1448,7 +1537,7 @@ the XSS protection rules defined in the OWASP XSS Prevention Cheat Sheet. security - Security - Blowfish Usage with Weak Key Size + Security - Blowfish usage with short key BLOWFISH_KEY_SIZE <p> The Blowfish cipher supports keysizes from 32 bits to 448 bits. A small key size makes the ciphertext vulnerable to brute force attacks. @@ -1479,9 +1568,9 @@ keyGen.init(128);</pre> security - Security - RSA Usage with Weak Key Size + Security - RSA usage with short key RSA_KEY_SIZE - <p>"RSA Laboratories currently recommends key sizes of 1024 bits for corporate use and 2048 bits for extremely valuable keys + <p>"RSA Laboratories currently recommends key sizes of 1024 bits for corporate use and 2048 bits for extremely valuable keys like the root key pair used by a certifying authority". <sup>[1]</sup></p> <p><b>Vulnerable Code:</b><br/> @@ -1558,52 +1647,6 @@ keyGen.initialize(2048); cwe security - - Security - Potential XSS in JSP - XSS_JSP_PRINT - <p>NOTE: If you want this rule to work, you have to precompile your JSPs (which usually isn't done in most build environments) -and then provide FindBugs the path information to the generated class files and the original JSPs themselves. If you are seeing -findings for this rule, you know you have your environment set up properly. Also, this XSS in JSP rule looks for similar issues, -but looks for them in a different way than the existing 'XSS: JSP reflected cross site scripting vulnerability' rule in FindBugs.</p> -<p>A potential XSS was found. It could be used to execute unwanted JavaScript in a client's browser. (See references) -</p> -<p> - <b>Vulnerable Code:</b> - <pre><% -String taintedInput = (String) request.getAttribute("input"); -%> -[...] -<%= taintedInput %></pre> -</p> -<p> - <b>Solution:</b> - <pre> -<% -String taintedInput = (String) request.getAttribute("input"); -%> -[...] -<%= Encode.forHtml(taintedInput) %> - </pre> -</p> -<p> -The best defense against XSS is context sensitive output encoding like the example above. There are typically 4 contexts to consider: -HTML, JavaScript, CSS (styles), and URLs. Please follow the XSS protection rules defined in the OWASP XSS Prevention Cheat Sheet, -which explains these defenses in significant detail. -</p> -<br/> -<p> -<b>References</b><br/> -<a href="http://projects.webappsec.org/w/page/13246920/Cross%20Site%20Scripting">WASC-8: Cross Site Scripting</a><br/> -<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">OWASP: XSS Prevention Cheat Sheet</a><br/> -<a href="https://www.owasp.org/index.php/Top_10_2013-A3-Cross-Site_Scripting_%28XSS%29">OWASP: Top 10 2013-A3: Cross-Site Scripting (XSS)</a><br/> -<a href="http://cwe.mitre.org/data/definitions/79.html">CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')</a><br/> -<a href="https://code.google.com/p/owasp-java-encoder/">OWASP Java Encoder</a><br/> -</p> - owasp-a3 - wasc - cwe - security - Security - Potential XSS in Servlet XSS_SERVLET @@ -1627,11 +1670,11 @@ A potential XSS was found. It could be used to execute unwanted JavaScript in a }</pre> </p> <p> -The best defense against XSS is context sensitive output encoding like the example above. There are typically 4 contexts to consider: +The best defense against XSS is context sensitive output encoding like the example above. There are typically 4 contexts to consider: HTML, JavaScript, CSS (styles), and URLs. Please follow the XSS protection rules defined in the OWASP XSS Prevention Cheat Sheet, which explains these defenses in significant detail. </p> -<p>Note that this XSS in Servlet rule looks for similar issues, but looks for them in a different way than the existing +<p>Note that this XSS in Servlet rule looks for similar issues, but looks for them in a different way than the existing 'XSS: Servlet reflected cross site scripting vulnerability' and 'XSS: Servlet reflected cross site scripting vulnerability in error page' rules in FindBugs. </p> <br/> @@ -1737,11 +1780,11 @@ public void encrypt(String message) throws Exception { security - Security - ECB Mode Unsafe + Security - ECB mode is insecure ECB_MODE - <p>An authentication cipher mode which provides better confidentiality of the encrypted data should be used instead of Electronic Codebook (ECB) mode, -which does not provide good confidentiality. Specifically, ECB mode produces the same output for the same input each time. So, -for example, if a user is sending a password, the encrypted value is the same each time. This allows an attacker to intercept + <p>An authentication cipher mode which provides better confidentiality of the encrypted data should be used instead of Electronic Codebook (ECB) mode, +which does not provide good confidentiality. Specifically, ECB mode produces the same output for the same input each time. So, +for example, if a user is sending a password, the encrypted value is the same each time. This allows an attacker to intercept and replay the data.</p> <p> To fix this, something like Galois/Counter Mode (GCM) should be used instead. @@ -1771,7 +1814,7 @@ byte[] cipherText = c.doFinal(plainText);</pre> security - Security - Cipher is Susceptible to Padding Oracle + Security - Cipher is susceptible to Padding Oracle PADDING_ORACLE <p> This specific mode of CBC with PKCS5Padding is susceptible to padding oracle attacks. An adversary could potentially decrypt the @@ -1805,14 +1848,14 @@ byte[] cipherText = c.doFinal(plainText);</pre> security - Security - Cipher With No Integrity + Security - Cipher with no integrity CIPHER_INTEGRITY <p> - The ciphertext produced is susceptible to alteration by an adversary. This mean that the cipher provides no way to detect that the + The ciphertext produced is susceptible to alteration by an adversary. This mean that the cipher provides no way to detect that the data has been tampered with. If the ciphertext can be controlled by an attacker, it could be altered without detection. </p> <p> - The solution is to used a cipher that includes a Hash based Message Authentication Code (HMAC) to sign the data. Combining a HMAC function to the + The solution is to used a cipher that includes a Hash based Message Authentication Code (HMAC) to sign the data. Combining a HMAC function to the existing cipher is prone to error <sup><a href="http://www.thoughtcrime.org/blog/the-cryptographic-doom-principle/">[1]</a></sup>. Specifically, it is always recommended that you be able to verify the HMAC first, and only if the data is unmodified, do you then perform any cryptographic functions on the data. @@ -1854,7 +1897,7 @@ In the example solution above, the GCM mode introduces an HMAC into the resultin security - Security - Usage of ESAPI Encryptor + Security - Use of ESAPI Encryptor ESAPI_ENCRYPTOR <p> The ESAPI has a small history of vulnerabilities within the cryptography component. Here is a quick validation list to @@ -1865,7 +1908,7 @@ In the example solution above, the GCM mode introduces an HMAC into the resultin This issue is corrected in ESAPI version 2.1.0. Versions <= 2.0.1 are vulnerable to a MAC bypass (CVE-2013-5679).<br/> </p> <p> - For Maven users, the plugin <a href="http://mojo.codehaus.org/versions-maven-plugin/">versions</a> can called using the + For Maven users, the plugin <a href="http://mojo.codehaus.org/versions-maven-plugin/">versions</a> can called using the following command. The effective version of ESAPI will be available in the output.<br/> <pre>$ mvn versions:display-dependency-updates</pre> <br/>Output:<br/> @@ -1932,7 +1975,7 @@ Encryptor.cipher_modes.additional_allowed=</pre> security - Security - External File Access (Android) + Security - External file access (Android) ANDROID_EXTERNAL_FILE_ACCESS <p> The application write data to external storage (potentially SD card). There are multiple security implication to this @@ -2043,7 +2086,7 @@ sendBroadcast(v1); security - Security - World Writable File (Android) + Security - World writable file (Android) ANDROID_WORLD_WRITABLE <p> The file written in this context is using the creation mode <code>MODE_WORLD_READABLE</code>. It might not be the @@ -2084,7 +2127,7 @@ create on external storage. See references below for implementation guidelines. security - Security - WebView with Geolocation Activated (Android) + Security - WebView with geolocation activated (Android) ANDROID_GEOLOCATION <p> It is suggest to ask the user for a confirmation about obtaining its geolocation. @@ -2126,7 +2169,7 @@ webView.setWebChromeClient(new WebChromeClient() { security - Security - WebView with JavaScript Enabled (Android) + Security - WebView with JavaScript enabled (Android) ANDROID_WEB_VIEW_JAVASCRIPT <p> Enabling JavaScript for the WebView means that it is now susceptible to XSS. The page render should be inspected @@ -2165,10 +2208,10 @@ function updateDescription(newDescription) { security - Security - WebView with Javascript Interface (Android) + Security - WebView with JavaScript interface (Android) ANDROID_WEB_VIEW_JAVASCRIPT_INTERFACE <p> - The use Javascript Interface could expose the WebView to risky API. If an XSS is trigger in the WebView, the class + The use JavaScript Interface could expose the WebView to risky API. If an XSS is trigger in the WebView, the class could be call by the malicious JavaScript code. </p> @@ -2254,6 +2297,53 @@ cookie.setHttpOnly(true); <a href="https://cwe.mitre.org/data/definitions/311.html">CWE-311: Missing Encryption of Sensitive Data</a><br/> <a href="https://www.owasp.org/index.php/SecureFlag">OWASP: Secure Flag</a><br/> <a href="https://www.rapid7.com/db/vulnerabilities/http-cookie-secure-flag">Rapid7: Missing Secure Flag From SSL Cookie</a> +</p> + cwe + security + + + Security - Object deserialization is used in {0}.{1} + OBJECT_DESERIALIZATION + <p> + Object deserialization of untrusted data can lead to remote code execution, if there is a class in classpath allow + the trigger of malicious operation. +</p> +<p> + Libraries developers tend to fix class that provided potential malicious trigger. Their are still classes that are + known to trigger Denial of Service<sup>[1]</sup>. +</p> +<p> + Deserialization is a sensible operation that has a great history of vulnerabilities. The web application might + become vulnerable has soon as a new vulnerability is found in the Java Virtual Machine<sup>[2] [3]</sup>. +</p> + +<p> +<b>Code at risk:</b><br/> +<pre> +public UserData deserializeObject(InputStream receivedFile) throws IOException, ClassNotFoundException { + + try (ObjectInputStream in = new ObjectInputStream(receivedFile)) { + return (UserData) in.readObject(); + } +} +</pre> +</p> + +<p> +<b>Solution:</b><br/> +<p> +Avoid deserializing object provided by remote users. +</p> +<br/> +<p> +<b>References</b><br/> +<a href="https://cwe.mitre.org/data/definitions/502.html">CWE-502: Deserialization of Untrusted Data</a><br/> +<a href="https://www.owasp.org/index.php/Deserialization_of_untrusted_data">Deserialization of untrusted data</a><br/> +<a href="http://www.oracle.com/technetwork/java/seccodeguide-139067.html#8">Serialization and Deserialization </a><br/> +<a href="https://github.com/frohoff/ysoserial">A proof-of-concept tool for generating payloads that exploit unsafe Java object deserialization</a><br/> +[1] <a href="https://gist.github.com/coekie/a27cc406fc9f3dc7a70d">Example of Denial of Service using the class java.util.HashSet</a><br/> +[2] <a href="https://bugzilla.redhat.com/show_bug.cgi?id=CVE-2015-2590">OpenJDK: Deserialization issue in ObjectInputStream.readSerialData() (CVE-2015-2590)</a><br/> +[3] <a href="https://www.rapid7.com/db/modules/exploit/multi/browser/java_calendar_deserialize">Rapid7: Sun Java Calendar Deserialization Privilege Escalation (CVE-2008-5353)</a> </p> cwe security diff --git a/src/main/resources/org/sonar/plugins/findbugs/rules-jsp.xml b/src/main/resources/org/sonar/plugins/findbugs/rules-jsp.xml new file mode 100644 index 00000000..dfd1f506 --- /dev/null +++ b/src/main/resources/org/sonar/plugins/findbugs/rules-jsp.xml @@ -0,0 +1,176 @@ + + + Security - JSP reflected cross site scripting vulnerability + XSS_REQUEST_PARAMETER_TO_JSP_WRITER + +<p>A potential XSS was found. It could be used to execute unwanted JavaScript in a client's browser. (See references) +</p> +<p> + <b>Vulnerable Code:</b> + <pre><% +String taintedInput = (String) request.getAttribute("input"); +%> +[...] +&lt;%= taintedInput %&gt;</pre> +</p> +<p> + <b>Solution:</b> + <pre> +&lt;% +String taintedInput = (String) request.getAttribute("input"); +%&gt; +[...] +&lt;%= Encode.forHtml(taintedInput) %&gt; + </pre> +</p> +<p> +The best defense against XSS is context sensitive output encoding like the example above. There are typically 4 contexts to consider: +HTML, JavaScript, CSS (styles), and URLs. Please follow the XSS protection rules defined in the OWASP XSS Prevention Cheat Sheet, +which explains these defenses in significant detail. +</p> +<br/> +<p> +<b>References</b><br/> +<a href="http://projects.webappsec.org/w/page/13246920/Cross%20Site%20Scripting">WASC-8: Cross Site Scripting</a><br/> +<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">OWASP: XSS Prevention Cheat Sheet</a><br/> +<a href="https://www.owasp.org/index.php/Top_10_2013-A3-Cross-Site_Scripting_%28XSS%29">OWASP: Top 10 2013-A3: Cross-Site Scripting (XSS)</a><br/> +<a href="http://cwe.mitre.org/data/definitions/79.html">CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')</a><br/> +<a href="https://code.google.com/p/owasp-java-encoder/">OWASP Java Encoder</a><br/> +</p> + + owasp-a3 + cwe + + + Security - Dynamic JSP inclusion + JSP_INCLUDE + <p>The inclusion of JSP file allow the entry of dynamic value. It may allow an attacker to control the JSP page included. +If this is the case, an attacker will try to include a file on disk that he controls. By including arbitrary files, the +attacker gets the ability to execute any code. +</p> +<p> + <b>Vulnerable Code:</b> + <pre>&lt;jsp:include page="${param.secret_param}" /&gt;</pre> +</p> +<p> + <b>Solution:</b> + <pre>&lt;c:if test="${param.secret_param == 'page1'}"&gt; + &lt;jsp:include page="page1.jsp" /&gt; +&lt;/c:if&gt;</pre> +</p> +<br/> +<p> +<b>References</b><br/> +<a href="http://resources.infosecinstitute.com/file-inclusion-attacks/">InfosecInstitute: File Inclusion Attacks</a><br/> +<a href="http://projects.webappsec.org/w/page/13246955/Remote%20File%20Inclusion">WASC-05: Remote File Inclusion</a><br/> +</p> + wasc + jsp + security + + + Security - Dynamic variable in Spring expression + JSP_SPRING_EVAL + <p></p> +<p> + <b>Vulnerable Code:</b> + <pre>&lt;%@ taglib prefix="spring" uri="http://www.springframework.org/tags" %&gt; + +&lt;spring:eval expression="${param.lang}" var="lang" /&gt;</pre> + <br> + <pre>&lt;%@ taglib prefix="spring" uri="http://www.springframework.org/tags" %&gt; + +&lt;spring:eval expression="'${param.lang}'=='fr'" var="languageIsFrench" /&gt;</pre> +</p> +<p> + <b>Solution:</b> + <pre>&lt;c:set var="lang" value="${param.lang}"/&gt;</pre> + <br/> + <pre>&lt;c:set var="languageIsFrench" value="${param.lang == 'fr'}"/&gt;</pre> +</p> +<br/> +<p> +<b>References</b><br/> +<a href="http://resources.infosecinstitute.com/file-inclusion-attacks/">InfosecInstitute: File Inclusion Attacks</a><br/> +<a href="http://projects.webappsec.org/w/page/13246955/Remote%20File%20Inclusion">WASC-05: Remote File Inclusion</a><br/> +</p> + wasc + jsp + security + + + Security - Escaping of special XML characters is disabled + JSP_JSTL_OUT + <p>A potential XSS was found. It could be used to execute unwanted JavaScript in a client's browser. (See references) +</p> +<p> + <b>Vulnerable Code:</b> + <pre>&lt;%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %&gt; + +&lt;c:out value="${param.test_param}" escapeXml="false"/&gt;</pre> +</p> +<p> + <b>Solution:</b> + <pre>&lt;%@ taglib prefix="c" uri="http://java.sun.com/jsp/jstl/core" %&gt; + +&lt;c:out value="${param.test_param}"/&gt;</pre> +</p> +<br/> +<p> +<b>References</b><br/> +<a href="http://projects.webappsec.org/w/page/13246920/Cross%20Site%20Scripting">WASC-8: Cross Site Scripting</a><br/> +<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">OWASP: XSS Prevention Cheat Sheet</a><br/> +<a href="https://www.owasp.org/index.php/Top_10_2013-A3-Cross-Site_Scripting_%28XSS%29">OWASP: Top 10 2013-A3: Cross-Site Scripting (XSS)</a><br/> +<a href="http://cwe.mitre.org/data/definitions/79.html">CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')</a><br/> +<a href="http://docs.oracle.com/javaee/5/jstl/1.1/docs/tlddocs/c/out.html">JSTL Javadoc: Out tag</a><br/> +</p> + owasp-a3 + wasc + cwe + jsp + security + + + Security - Potential XSS in JSP + XSS_JSP_PRINT + <p>A potential XSS was found. It could be used to execute unwanted JavaScript in a client's browser. (See references) +</p> +<p> + <b>Vulnerable Code:</b> + <pre><% +String taintedInput = (String) request.getAttribute("input"); +%> +[...] +&lt;%= taintedInput %&gt;</pre> +</p> +<p> + <b>Solution:</b> + <pre> +&lt;% +String taintedInput = (String) request.getAttribute("input"); +%&gt; +[...] +&lt;%= Encode.forHtml(taintedInput) %&gt; + </pre> +</p> +<p> +The best defense against XSS is context sensitive output encoding like the example above. There are typically 4 contexts to consider: +HTML, JavaScript, CSS (styles), and URLs. Please follow the XSS protection rules defined in the OWASP XSS Prevention Cheat Sheet, +which explains these defenses in significant detail. +</p> +<br/> +<p> +<b>References</b><br/> +<a href="http://projects.webappsec.org/w/page/13246920/Cross%20Site%20Scripting">WASC-8: Cross Site Scripting</a><br/> +<a href="https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet">OWASP: XSS Prevention Cheat Sheet</a><br/> +<a href="https://www.owasp.org/index.php/Top_10_2013-A3-Cross-Site_Scripting_%28XSS%29">OWASP: Top 10 2013-A3: Cross-Site Scripting (XSS)</a><br/> +<a href="http://cwe.mitre.org/data/definitions/79.html">CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')</a><br/> +<a href="https://code.google.com/p/owasp-java-encoder/">OWASP Java Encoder</a><br/> +</p> + owasp-a3 + wasc + cwe + jsp + security + + \ No newline at end of file diff --git a/src/main/resources/org/sonar/plugins/findbugs/rules.xml b/src/main/resources/org/sonar/plugins/findbugs/rules.xml index be1b7509..277000bc 100644 --- a/src/main/resources/org/sonar/plugins/findbugs/rules.xml +++ b/src/main/resources/org/sonar/plugins/findbugs/rules.xml @@ -158,14 +158,6 @@ cwe - - CRITICAL - - - owasp-a3 - cwe - - MAJOR diff --git a/src/test/java/org/sonar/plugins/findbugs/FakeRuleFinder.java b/src/test/java/org/sonar/plugins/findbugs/FakeRuleFinder.java index 145376c7..19fa7d95 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FakeRuleFinder.java +++ b/src/test/java/org/sonar/plugins/findbugs/FakeRuleFinder.java @@ -45,7 +45,7 @@ public class FakeRuleFinder { private FakeRuleFinder() { } - private static RuleFinder create(boolean findbugs, boolean fbContrib, boolean findSecBug) { + private static RuleFinder create(boolean findbugs, boolean fbContrib, boolean findSecBug, boolean findSecBugJsp) { RuleFinder ruleFinder = mock(RuleFinder.class); RulesDefinition.Context context = new RulesDefinition.Context(); @@ -55,34 +55,41 @@ private static RuleFinder create(boolean findbugs, boolean fbContrib, boolean fi configRuleFinderForRepo(ruleFinder, context, FindbugsRulesDefinition.REPOSITORY_KEY); } + if (fbContrib) { + RulesDefinition rulesDefinition = new FbContribRulesDefinition(); + rulesDefinition.define(context); + configRuleFinderForRepo(ruleFinder, context, FbContribRulesDefinition.REPOSITORY_KEY); + } + if (findSecBug) { RulesDefinition rulesDefinition = new FindSecurityBugsRulesDefinition(); rulesDefinition.define(context); configRuleFinderForRepo(ruleFinder, context, FindSecurityBugsRulesDefinition.REPOSITORY_KEY); } - if (fbContrib) { - RulesDefinition rulesDefinition = new FbContribRulesDefinition(); + if (findSecBugJsp) { + RulesDefinition rulesDefinition = new FindSecurityBugsJspRulesDefinition(); rulesDefinition.define(context); - configRuleFinderForRepo(ruleFinder, context, FbContribRulesDefinition.REPOSITORY_KEY); + configRuleFinderForRepo(ruleFinder, context, FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY); } + return ruleFinder; } public static RuleFinder createWithAllRules() { - return create(true, true, true); + return create(true, true, true, true); } public static RuleFinder createWithOnlyFindbugsRules() { - return create(true, false, false); + return create(true, false, false,false); } public static RuleFinder createWithOnlyFbContribRules() { - return create(false, true, false); + return create(false, true, false,false); } public static RuleFinder createWithOnlyFindSecBugsRules() { - return create(false, false, true); + return create(false, false, true,false); } private static void configRuleFinderForRepo(RuleFinder ruleFinder, final Context context, final String repositoryKey) { diff --git a/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java b/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java index a4635dcc..434d7ab8 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindSecurityBugsRulesDefinitionTest.java @@ -40,7 +40,7 @@ public void testLoadRepositoryFromXml() { assertThat(repository.language()).isEqualTo(Java.KEY); List rules = repository.rules(); - assertThat(rules).hasSize(66); + assertThat(rules).hasSize(69); for (Rule rule : rules) { assertThat(rule.key()).isNotNull(); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java index ecdd0f3e..01be3394 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsConfigurationTest.java @@ -119,16 +119,6 @@ public void should_set_class_files() throws IOException { conf.stop(); } - @Test - public void should_set_source_files() throws IOException { - File file = temp.newFile("MyClass.java"); - fs.add(new DefaultInputFile(file.getPath()).setAbsolutePath(file.getAbsolutePath()).setType(Type.MAIN).setLanguage(Java.KEY)); - Project findbugsProject = conf.getFindbugsProject(); - - assertThat(findbugsProject.getFileList()).containsOnly(file.getCanonicalPath()); - conf.stop(); - } - @Test public void should_set_class_path() throws IOException { File classpath = temp.newFolder(); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java index ee7fb955..3a394d60 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsPluginTest.java @@ -27,7 +27,7 @@ public class FindbugsPluginTest { @Test public void testGetExtensions() { - assertThat(new FindbugsPlugin().getExtensions()).hasSize(15); + assertThat(new FindbugsPlugin().getExtensions()).hasSize(20); } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java index 78faa6e1..2ffa0f9e 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileImporterTest.java @@ -99,7 +99,7 @@ public void shouldImportConfigurationBugInclude() { RulesProfile profile = importer.importProfile(new InputStreamReader(input), ValidationMessages.create()); List results = profile.getActiveRules(); - assertThat(results).hasSize(11); + assertThat(results).hasSize(10); assertThat(profile.getActiveRule(FindbugsRulesDefinition.REPOSITORY_KEY, "RC_REF_COMPARISON_BAD_PRACTICE")).isNotNull(); } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileTest.java index 765989dc..f923ff99 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsProfileTest.java @@ -34,7 +34,7 @@ public void shouldCreateProfile() { ValidationMessages validation = ValidationMessages.create(); RulesProfile profile = findbugsProfile.createProfile(validation); assertThat(profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY)) - .hasSize(379); + .hasSize(378); assertThat(validation.hasErrors()).isFalse(); } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java index aed6d19c..1f92f7ce 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsRulesDefinitionTest.java @@ -41,7 +41,7 @@ public void test() { assertThat(repository.language()).isEqualTo(Java.KEY); List rules = repository.rules(); - assertThat(rules).hasSize(442); + assertThat(rules).hasSize(441); List rulesWithMissingSQALE = Lists.newLinkedList(); for (Rule rule : rules) { diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityAuditProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityAuditProfileTest.java index 5dd5e1e1..e22bcca6 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityAuditProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityAuditProfileTest.java @@ -34,8 +34,8 @@ public void shouldCreateProfile() { ValidationMessages validation = ValidationMessages.create(); RulesProfile profile = secOnlyProfile.createProfile(validation); // The standard FindBugs include only 9. Fb-Contrib and FindSecurityBugs include other rules - assertThat(profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY)).hasSize(9); - assertThat(profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)).hasSize(66); + assertThat(profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY)).hasSize(8); + assertThat(profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)).hasSize(65); assertThat(validation.hasErrors()).isFalse(); } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfileTest.java new file mode 100644 index 00000000..9f6ebe92 --- /dev/null +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityJspProfileTest.java @@ -0,0 +1,43 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs; + +import org.junit.Test; +import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.utils.ValidationMessages; + +import static org.fest.assertions.Assertions.assertThat; + +/** + * Created by Philippe on 2015-11-23. + */ +public class FindbugsSecurityJspProfileTest { + + @Test + public void shouldCreateProfile() { + FindbugsProfileImporter importer = new FindbugsProfileImporter(FakeRuleFinder.createWithAllRules()); + FindbugsSecurityJspProfile secJspProfile = new FindbugsSecurityJspProfile(importer); + ValidationMessages validation = ValidationMessages.create(); + RulesProfile profile = secJspProfile.createProfile(validation); + + assertThat(profile.getActiveRulesByRepository(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY)).hasSize(5); + assertThat(validation.hasErrors()).isFalse(); + } +} diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityMinimalProfileTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityMinimalProfileTest.java index 76854fa1..395219e2 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityMinimalProfileTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSecurityMinimalProfileTest.java @@ -34,9 +34,9 @@ public void shouldCreateProfile() { ValidationMessages validation = ValidationMessages.create(); RulesProfile profile = secOnlyProfile.createProfile(validation); // The standard FindBugs include only 9. Fb-Contrib and FindSecurityBugs include other rules - assertThat(profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY)).hasSize(9); + assertThat(profile.getActiveRulesByRepository(FindbugsRulesDefinition.REPOSITORY_KEY)).hasSize(8); // 62 rules total - 20 informational = 42 major or critical - assertThat(profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)).hasSize(42); + assertThat(profile.getActiveRulesByRepository(FindSecurityBugsRulesDefinition.REPOSITORY_KEY)).hasSize(41); assertThat(validation.hasErrors()).isFalse(); } } diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java index 3fe89daf..30b61b2d 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsSensorTest.java @@ -38,6 +38,7 @@ import org.sonar.api.resources.ProjectFileSystem; import org.sonar.api.resources.Resource; import org.sonar.api.rule.RuleKey; +import org.sonar.plugins.findbugs.resource.ByteCodeResourceLocator; import org.sonar.plugins.java.api.JavaResourceLocator; import java.io.File; @@ -63,6 +64,7 @@ public class FindbugsSensorTest extends FindbugsTests { private SensorContext context; private ResourcePerspectives perspectives; private Issuable issuable; + private ByteCodeResourceLocator byteCodeResourceLocator; @Before public void setUp() { @@ -80,11 +82,12 @@ public void setUp() { when(issueBuilder.build()).thenReturn(mock(Issue.class)); when(issuable.newIssueBuilder()).thenReturn(issueBuilder); when(perspectives.as(eq(Issuable.class), any(Resource.class))).thenReturn(issuable); + byteCodeResourceLocator = new ByteCodeResourceLocator(); } @Test public void shouldNotAnalyseIfJavaProjectButNoSource() { - FindbugsSensor sensor = new FindbugsSensor(null, null, perspectives, null, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(null, null, perspectives, null, mockJavaResourceLocator(), fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isFalse(); } @@ -97,35 +100,35 @@ private void addJavaFileToFs() { @Test public void shouldNotAnalyseIfJavaProjectButNoRules() { addJavaFileToFs(); - FindbugsSensor sensor = new FindbugsSensor(RulesProfile.create(), null, perspectives, null, null, fs); + FindbugsSensor sensor = new FindbugsSensor(RulesProfile.create(), null, perspectives, null, null, fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isFalse(); } @Test public void shouldAnalyse() { addJavaFileToFs(); - FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(), null, perspectives, null, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(), null, perspectives, null, mockJavaResourceLocator(), fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isTrue(); } @Test public void should_analyse_if_fbContrib_and_FindSecBug() { addJavaFileToFs(); - FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, true, true), null, perspectives, null, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, true, true,false), null, perspectives, null, mockJavaResourceLocator(), fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isTrue(); } @Test public void should_analyse_if_FindSecBug() { addJavaFileToFs(); - FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, true), null, perspectives, null, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, true,false), null, perspectives, null, mockJavaResourceLocator(), fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isTrue(); } @Test public void should_not_analyze_if_not_rules() { addJavaFileToFs(); - FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, false), null, perspectives, null, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, false,false), null, perspectives, null, mockJavaResourceLocator(), fs, byteCodeResourceLocator); assertThat(sensor.shouldExecuteOnProject(project)).isFalse(); } @@ -140,7 +143,7 @@ public void should_execute_findbugs() throws Exception { JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs, byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(false, false); @@ -159,7 +162,7 @@ public void should_not_add_issue_if_resource_not_found() throws Exception { when(javaResourceLocator.findResourceByClassName(anyString())).thenReturn(null); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs, byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(false, false); @@ -178,7 +181,7 @@ public void should_not_add_issue_if_issuable_not_found() throws Exception { when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); when(perspectives.as(eq(Issuable.class), any(Resource.class))).thenReturn(null); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, fs, byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(false, false); @@ -196,8 +199,8 @@ public void should_execute_findbugs_even_if_only_fbcontrib() throws Exception { JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(false, true, false), FakeRuleFinder.createWithAllRules(), perspectives, executor, - javaResourceLocator, fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(false, true, false, false), FakeRuleFinder.createWithAllRules(), perspectives, executor, + javaResourceLocator, fs, byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(true, false); @@ -216,12 +219,13 @@ public void should_execute_findbugs_even_if_only_findsecbug() throws Exception { when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); FindbugsSensor analyser = new FindbugsSensor( - createRulesProfileWithActiveRules(false, false, true), + createRulesProfileWithActiveRules(false, false, true, false), FakeRuleFinder.createWithAllRules(), perspectives, executor, javaResourceLocator, - fs); + fs, + byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(false, true); @@ -239,8 +243,9 @@ public void should_execute_findbugs_but_not_find_violation() throws Exception { JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Lists.newArrayList(new File("file"))); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, false), FakeRuleFinder.createWithAllRules(), perspectives, executor, - javaResourceLocator, fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(false, false, false, false), + FakeRuleFinder.createWithAllRules(), perspectives, executor, + javaResourceLocator, fs,byteCodeResourceLocator); analyser.analyse(project, context); verify(executor).execute(false, false); @@ -267,7 +272,8 @@ public void should_not_execute_if_no_compiled_class_available() throws Exception JavaResourceLocator javaResourceLocator = mockJavaResourceLocator(); when(javaResourceLocator.classFilesToAnalyze()).thenReturn(Collections.emptyList()); - FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(), null, perspectives, executor, mockJavaResourceLocator(), fs); + FindbugsSensor sensor = new FindbugsSensor(createRulesProfileWithActiveRules(), null, perspectives, executor, + mockJavaResourceLocator(), fs, byteCodeResourceLocator); sensor.analyse(project, context); verify(executor, never()).execute(); @@ -288,7 +294,8 @@ public void shouldIgnoreNotActiveViolations() throws Exception { Collection collection = Arrays.asList(new ReportedBug(bugInstance)); when(executor.execute()).thenReturn(collection); - FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), perspectives, executor, mockJavaResourceLocator(), fs); + FindbugsSensor analyser = new FindbugsSensor(createRulesProfileWithActiveRules(), FakeRuleFinder.createWithAllRules(), + perspectives, executor, mockJavaResourceLocator(), fs, byteCodeResourceLocator); analyser.analyse(project, context); verify(issuable, never()).addIssue(any(Issue.class)); diff --git a/src/test/java/org/sonar/plugins/findbugs/FindbugsTests.java b/src/test/java/org/sonar/plugins/findbugs/FindbugsTests.java index cd69f8d7..5936600a 100644 --- a/src/test/java/org/sonar/plugins/findbugs/FindbugsTests.java +++ b/src/test/java/org/sonar/plugins/findbugs/FindbugsTests.java @@ -68,7 +68,8 @@ protected List buildActiveRulesFixture(List rules) { return activeRules; } - protected RulesProfile createRulesProfileWithActiveRules(boolean findbugs, boolean fbContrib, boolean findsecbug) { + protected RulesProfile createRulesProfileWithActiveRules(boolean findbugs, boolean fbContrib, boolean findsecbug, + boolean findbugsJsp) { RulesProfile profile = RulesProfile.create(); profile.setName("FindBugs"); profile.setLanguage(Java.KEY); @@ -89,11 +90,17 @@ protected RulesProfile createRulesProfileWithActiveRules(boolean findbugs, boole profile.activateRule(rule, null); } } + if (findbugsJsp) { + for (Rule rule : ruleFinder.findAll(RuleQuery.create().withRepositoryKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY))) { + rule.setRepositoryKey(FindSecurityBugsJspRulesDefinition.REPOSITORY_KEY); + profile.activateRule(rule, null); + } + } return profile; } protected RulesProfile createRulesProfileWithActiveRules() { - return createRulesProfileWithActiveRules(true, false, false); + return createRulesProfileWithActiveRules(true, false, false, false); } private void assertSimilarXml(File expectedFile, String xml) throws SAXException, IOException { diff --git a/src/test/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractorTest.java b/src/test/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractorTest.java new file mode 100644 index 00000000..3fca1ee6 --- /dev/null +++ b/src/test/java/org/sonar/plugins/findbugs/resource/DebugExtensionExtractorTest.java @@ -0,0 +1,52 @@ +/* + * SonarQube Findbugs Plugin + * Copyright (C) 2012 SonarSource + * sonarqube@googlegroups.com + * + * 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 02 + */ +package org.sonar.plugins.findbugs.resource; + +import org.junit.Test; + +import java.io.IOException; +import java.io.InputStream; + +import static org.fest.assertions.Assertions.assertThat; + +public class DebugExtensionExtractorTest { + + @Test + public void loadDebugInfoFromWeblogicClass() throws IOException { + InputStream in = getClass().getResourceAsStream("/jsp_classes/weblogic/__test.clazz"); + String debugInfo = new DebugExtensionExtractor().getDebugExtFromClass(in); + //System.out.println(debugInfo); + + SmapParser smap = new SmapParser(debugInfo); + int[] jspLines = smap.getScriptLineNumber(282); + assertThat(jspLines[1]).isEqualTo(20); + } + + @Test + public void loadDebugInfoFromJettyClass() throws IOException { + InputStream in = getClass().getResourceAsStream("/jsp_classes/jetty936/test_jsp.clazz"); + String debugInfo = new DebugExtensionExtractor().getDebugExtFromClass(in); + //System.out.println(debugInfo); + + SmapParser smap = new SmapParser(debugInfo); + int[] jspLines = smap.getScriptLineNumber(260); + assertThat(jspLines[1]).isEqualTo(20); + } +} diff --git a/src/test/resources/jsp_classes/jetty936/smap.txt b/src/test/resources/jsp_classes/jetty936/smap.txt new file mode 100644 index 00000000..38fc0250 --- /dev/null +++ b/src/test/resources/jsp_classes/jetty936/smap.txt @@ -0,0 +1,25 @@ +SMAP +test_jsp.java +JSP +*S JSP +*F ++ 0 test.jsp +test.jsp ++ 1 index.jsp +index.jsp +*L +1,6:127,0 +6:172,6 +6,3:130,0 +8:191,8 +8,3:133,0 +10:212,8 +10,5:136,0 +1#1,23:137,0 +14#0,3:138,0 +16:233,8 +16,5:141,0 +20:260,10 +20:144,0 +21:289,6 +*E diff --git a/src/test/resources/jsp_classes/jetty936/test_jsp.clazz b/src/test/resources/jsp_classes/jetty936/test_jsp.clazz new file mode 100644 index 00000000..aeab4a40 Binary files /dev/null and b/src/test/resources/jsp_classes/jetty936/test_jsp.clazz differ diff --git a/src/test/resources/jsp_classes/weblogic/__test.clazz b/src/test/resources/jsp_classes/weblogic/__test.clazz new file mode 100644 index 00000000..c1e2c0ec Binary files /dev/null and b/src/test/resources/jsp_classes/weblogic/__test.clazz differ diff --git a/src/test/resources/jsp_classes/weblogic/smap.txt b/src/test/resources/jsp_classes/weblogic/smap.txt new file mode 100644 index 00000000..3dfe2412 --- /dev/null +++ b/src/test/resources/jsp_classes/weblogic/smap.txt @@ -0,0 +1,38 @@ +SMAP +__test.java +JSP +*S JSP +*F +1 test.jsp ++ 2 index.jsp +index.jsp +*L +4#1:90 +6#1:92,2 +8#1:96,2 +10#1:100,2 +12#1:102 +1#2:103 +16#1:106,2 +18#1:108 +20#1:110,2 +21#1:114,13 +6#1:128,7 +6#1:136,9 +6#1:146,14 +8#1:161,7 +8#1:169,10 +8#1:180,14 +10#1:195,7 +10#1:203,10 +10#1:214,14 +16#1:229,8 +16#1:238,9 +16#1:248,24 +20#1:273,8 +20#1:282,10 +20#1:293,25 +21#1:319,7 +21#1:327,9 +21#1:337,15 +*E