From 36692ac10ddff11b78b1bab3480ea64f8675bfc0 Mon Sep 17 00:00:00 2001 From: Maarten Billemont Date: Tue, 24 Sep 2019 11:19:21 -0400 Subject: [PATCH] Consistent SpotBugs configuration & warning fixes. --- build.gradle | 43 +++++++++++-------- platform-android/build.gradle | 1 - .../java/algorithm/build.gradle | 2 +- .../lyndir/masterpassword/impl/Native.java | 2 + platform-independent/java/gui/build.gradle | 2 +- .../lyndir/masterpassword/gui/util/Res.java | 2 + .../gui/view/UserContentPanel.java | 3 +- platform-independent/java/model/build.gradle | 3 +- .../masterpassword/model/impl/MPFileUser.java | 3 ++ platform-independent/java/tests/build.gradle | 2 +- .../lyndir/masterpassword/MPTestSuite.java | 30 ++++++++++--- .../masterpassword/MPMasterKeyTest.java | 3 +- 12 files changed, 62 insertions(+), 34 deletions(-) diff --git a/build.gradle b/build.gradle index 053fe1ff..85bf297a 100644 --- a/build.gradle +++ b/build.gradle @@ -1,38 +1,43 @@ -allprojects { - apply plugin: 'findbugs' - - group = 'com.lyndir.masterpassword' - version = '2.7.10' - - tasks.withType( JavaCompile ) { - options.encoding = 'UTF-8' - sourceCompatibility = '1.8' - targetCompatibility = '1.8' - } - tasks.withType( FindBugs ) { - reports { - xml.enabled = false - html.enabled = true - } - } -} - buildscript { repositories { google() jcenter() + gradlePluginPortal() } dependencies { classpath group: 'com.android.tools.build', name: 'gradle', version: '3.5.0' + classpath group: 'gradle.plugin.com.github.spotbugs', name: 'spotbugs-gradle-plugin', version: '2.0.0' } } +allprojects { + group = 'com.lyndir.masterpassword' + version = '2.7.10' +} + subprojects { + apply plugin: 'com.github.spotbugs' + repositories { google() jcenter() mavenCentral() maven { url 'https://maven.lyndir.com' } } + dependencies { + spotbugsPlugins group: 'com.h3xstream.findsecbugs', name: 'findsecbugs-plugin', version: '1.9.0' + } + + tasks.withType( JavaCompile ) { + options.encoding = 'UTF-8' + sourceCompatibility = '1.8' + targetCompatibility = '1.8' + } + tasks.withType( com.github.spotbugs.SpotBugsTask ) { + reports { + xml.enabled = false + html.enabled = true + } + } } diff --git a/platform-android/build.gradle b/platform-android/build.gradle index cf62e3eb..05cd166e 100644 --- a/platform-android/build.gradle +++ b/platform-android/build.gradle @@ -1,6 +1,5 @@ plugins { id 'com.android.application' - id 'com.github.spotbugs' version '2.0.0' } android { diff --git a/platform-independent/java/algorithm/build.gradle b/platform-independent/java/algorithm/build.gradle index d72079fc..7bb74257 100644 --- a/platform-independent/java/algorithm/build.gradle +++ b/platform-independent/java/algorithm/build.gradle @@ -1,6 +1,5 @@ plugins { id 'java-library' - id 'com.github.spotbugs' version '2.0.0' } description = 'Master Password Algorithm Implementation' @@ -16,6 +15,7 @@ configurations { dependencies { implementation group: 'com.lyndir.lhunath.opal', name: 'opal-system', version: '1.7-p2' + implementation group: 'com.github.spotbugs', name: 'spotbugs-annotations', version: '4.0.0-beta4' api group: 'com.fasterxml.jackson.core', name: 'jackson-annotations', version: '2.9.8' api group: 'org.jetbrains', name: 'annotations', version: '16.0.2' diff --git a/platform-independent/java/algorithm/src/main/java/com/lyndir/masterpassword/impl/Native.java b/platform-independent/java/algorithm/src/main/java/com/lyndir/masterpassword/impl/Native.java index e4dac85c..9b2d93dd 100644 --- a/platform-independent/java/algorithm/src/main/java/com/lyndir/masterpassword/impl/Native.java +++ b/platform-independent/java/algorithm/src/main/java/com/lyndir/masterpassword/impl/Native.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.io.ByteStreams; import com.lyndir.lhunath.opal.system.logging.Logger; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.*; import java.util.*; import java.util.function.Predicate; @@ -42,6 +43,7 @@ public final class Native { private static final char EXTENSION_SEPARATOR = '.'; private static final String NATIVES_PATH = "lib"; + @SuppressFBWarnings("PATH_TRAVERSAL_IN") @SuppressWarnings({ "HardcodedFileSeparator", "LoadLibraryWithNonConstantString" }) public static boolean load(final Class context, final String name) { diff --git a/platform-independent/java/gui/build.gradle b/platform-independent/java/gui/build.gradle index fd787c87..4bfcdd32 100644 --- a/platform-independent/java/gui/build.gradle +++ b/platform-independent/java/gui/build.gradle @@ -1,7 +1,6 @@ plugins { id 'java' id 'application' - id 'com.github.spotbugs' version '2.0.0' id 'com.github.johnrengelman.shadow' version '5.1.0' } @@ -13,6 +12,7 @@ dependencies { implementation group: 'ch.qos.logback', name: 'logback-classic', version: '1.1.2' implementation group: 'com.yuvimasory', name: 'orange-extensions', version: '1.3.0' implementation group: 'com.github.tulskiy', name: 'jkeymaster', version: '1.2' + implementation group: 'com.github.spotbugs', name: 'spotbugs-annotations', version: '4.0.0-beta4' compile project( ':masterpassword-model' ) } diff --git a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/util/Res.java b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/util/Res.java index 2f9e7671..c977f658 100644 --- a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/util/Res.java +++ b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/util/Res.java @@ -24,6 +24,7 @@ import com.google.common.io.Resources; import com.google.common.util.concurrent.*; import com.lyndir.lhunath.opal.system.logging.Logger; import com.lyndir.masterpassword.MPIdenticon; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.awt.*; import java.io.IOException; import java.util.concurrent.*; @@ -232,6 +233,7 @@ public abstract class Res { return new Font( fontName, style, size ); } + @SuppressFBWarnings("URLCONNECTION_SSRF_FD") private void register() { try { Font font = Font.createFont( Font.TRUETYPE_FONT, Resources.getResource( resourceName ).openStream() ); diff --git a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java index 21714b15..c2a2d34e 100644 --- a/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java +++ b/platform-independent/java/gui/src/main/java/com/lyndir/masterpassword/gui/view/UserContentPanel.java @@ -23,6 +23,7 @@ import java.io.File; import java.io.IOException; import java.nio.file.Files; import java.nio.file.StandardCopyOption; +import java.security.SecureRandom; import java.util.*; import java.util.Optional; import java.util.concurrent.Future; @@ -42,7 +43,7 @@ import javax.swing.text.PlainDocument; @SuppressWarnings("SerializableStoresNonSerializable") public class UserContentPanel extends JPanel implements MasterPassword.Listener, MPUser.Listener { - private static final Random random = new Random(); + private static final Random random = new SecureRandom(); private static final int SIZE_RESULT = 48; private static final Logger logger = Logger.get( UserContentPanel.class ); private static final JButton iconButton = Components.button( Res.icons().user(), null, null ); diff --git a/platform-independent/java/model/build.gradle b/platform-independent/java/model/build.gradle index c1ea7580..e51d95ba 100644 --- a/platform-independent/java/model/build.gradle +++ b/platform-independent/java/model/build.gradle @@ -1,6 +1,5 @@ plugins { id 'java-library' - id 'com.github.spotbugs' version '2.0.0' } description = 'Master Password Site Model' @@ -8,7 +7,7 @@ description = 'Master Password Site Model' dependencies { implementation group: 'com.lyndir.lhunath.opal', name: 'opal-system', version: '1.7-p2' implementation group: 'com.fasterxml.jackson.core', name: 'jackson-core', version: '2.9.8' - implementation 'com.github.spotbugs:spotbugs-annotations:4.0.0-beta4' + implementation group: 'com.github.spotbugs', name: 'spotbugs-annotations', version: '4.0.0-beta4' api project( ':masterpassword-algorithm' ) api group: 'joda-time', name: 'joda-time', version: '2.10' diff --git a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java index d05ece6d..94e0b249 100755 --- a/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java +++ b/platform-independent/java/model/src/main/java/com/lyndir/masterpassword/model/impl/MPFileUser.java @@ -21,6 +21,7 @@ package com.lyndir.masterpassword.model.impl; import com.lyndir.lhunath.opal.system.logging.Logger; import com.lyndir.masterpassword.*; import com.lyndir.masterpassword.model.*; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.File; import java.io.IOException; import javax.annotation.Nonnull; @@ -66,6 +67,7 @@ public class MPFileUser extends MPBasicUser { MPMarshaller.ContentMode.PROTECTED, MPMarshalFormat.DEFAULT, location ); } + @SuppressFBWarnings("PATH_TRAVERSAL_IN") public MPFileUser(final String fullName, @Nullable final byte[] keyID, final MPAlgorithm algorithm, final int avatar, @Nullable final MPResultType defaultType, final ReadableInstant lastUsed, final boolean hidePasswords, final MPMarshaller.ContentMode contentMode, final MPMarshalFormat format, final File location) { @@ -169,6 +171,7 @@ public class MPFileUser extends MPBasicUser { * The user's old file is either moved to the new or deleted. If the user's file was already at the destination, it doesn't change. * If a file already exists at the destination, it is overwritten. */ + @SuppressFBWarnings("PATH_TRAVERSAL_IN") public void migrateTo(final File path, final MPMarshalFormat newFormat) { MPMarshalFormat oldFormat = format; File oldFile = file, newFile = new File( path, getFullName() + newFormat.fileSuffix() ); diff --git a/platform-independent/java/tests/build.gradle b/platform-independent/java/tests/build.gradle index 45a35af4..2d82d9af 100644 --- a/platform-independent/java/tests/build.gradle +++ b/platform-independent/java/tests/build.gradle @@ -1,6 +1,5 @@ plugins { id 'java' - id 'com.github.spotbugs' version '2.0.0' } description = 'Master Password Test Suite' @@ -8,6 +7,7 @@ description = 'Master Password Test Suite' dependencies { implementation group: 'com.lyndir.lhunath.opal', name: 'opal-system', version: '1.7-p2' implementation group: 'javax.xml.bind', name: 'jaxb-api', version: '2.3.1' + implementation group: 'com.github.spotbugs', name: 'spotbugs-annotations', version: '4.0.0-beta4' implementation project( ':masterpassword-algorithm' ) implementation project( ':masterpassword-model' ) diff --git a/platform-independent/java/tests/src/main/java/com/lyndir/masterpassword/MPTestSuite.java b/platform-independent/java/tests/src/main/java/com/lyndir/masterpassword/MPTestSuite.java index dc31fd64..4fbdc3ee 100644 --- a/platform-independent/java/tests/src/main/java/com/lyndir/masterpassword/MPTestSuite.java +++ b/platform-independent/java/tests/src/main/java/com/lyndir/masterpassword/MPTestSuite.java @@ -23,13 +23,15 @@ import com.google.common.collect.Lists; import com.google.common.primitives.UnsignedInteger; import com.lyndir.lhunath.opal.system.logging.Logger; import com.lyndir.lhunath.opal.system.util.ConversionUtils; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; +import java.io.InputStream; import java.net.URL; import java.util.*; import java.util.concurrent.Callable; +import javax.xml.XMLConstants; import javax.xml.parsers.*; -import org.xml.sax.Attributes; -import org.xml.sax.SAXException; +import org.xml.sax.*; import org.xml.sax.ext.DefaultHandler2; @@ -40,8 +42,18 @@ import org.xml.sax.ext.DefaultHandler2; public class MPTestSuite implements Callable { @SuppressWarnings("UnusedDeclaration") - private static final Logger logger = Logger.get( MPTestSuite.class ); - private static final String DEFAULT_RESOURCE_NAME = "mpw_tests.xml"; + private static final Logger logger = Logger.get( MPTestSuite.class ); + private static final String DEFAULT_RESOURCE_NAME = "mpw_tests.xml"; + private static final SAXParserFactory factory = SAXParserFactory.newInstance(); + + static { + try { + factory.setFeature( XMLConstants.FEATURE_SECURE_PROCESSING, true ); + } + catch (ParserConfigurationException | SAXNotRecognizedException | SAXNotSupportedException e) { + throw new UnsupportedOperationException( e ); + } + } private final MPTests tests; private Listener listener; @@ -51,14 +63,18 @@ public class MPTestSuite implements Callable { this( DEFAULT_RESOURCE_NAME ); } + @SuppressFBWarnings("XXE_SAXPARSER") public MPTestSuite(final String resourceName) throws UnavailableException { try { tests = new MPTests(); tests.cases = Lists.newLinkedList(); - SAXParser parser = SAXParserFactory.newInstance().newSAXParser(); - Enumeration resources = Thread.currentThread().getContextClassLoader().getResources( "." ); - parser.parse( Thread.currentThread().getContextClassLoader().getResourceAsStream( resourceName ), new DefaultHandler2() { + SAXParser parser = factory.newSAXParser(); + InputStream resourceStream = Thread.currentThread().getContextClassLoader().getResourceAsStream( resourceName ); + if (resourceStream == null) + throw new UnavailableException( new NullPointerException( "Missing resource: " + resourceName ) ); + + parser.parse( resourceStream, new DefaultHandler2() { private final Deque currentTags = Lists.newLinkedList(); private final Deque currentTexts = Lists.newLinkedList(); private MPTests.Case currentCase; diff --git a/platform-independent/java/tests/src/test/java/com/lyndir/masterpassword/MPMasterKeyTest.java b/platform-independent/java/tests/src/test/java/com/lyndir/masterpassword/MPMasterKeyTest.java index 181e459c..ef692f49 100644 --- a/platform-independent/java/tests/src/test/java/com/lyndir/masterpassword/MPMasterKeyTest.java +++ b/platform-independent/java/tests/src/test/java/com/lyndir/masterpassword/MPMasterKeyTest.java @@ -22,6 +22,7 @@ import static org.testng.Assert.*; import com.lyndir.lhunath.opal.system.CodeUtils; import com.lyndir.lhunath.opal.system.logging.Logger; +import java.security.SecureRandom; import java.util.Random; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -31,6 +32,7 @@ public class MPMasterKeyTest { @SuppressWarnings("UnusedDeclaration") private static final Logger logger = Logger.get( MPMasterKeyTest.class ); + private static final Random random = new SecureRandom(); private MPTestSuite testSuite; @@ -121,7 +123,6 @@ public class MPMasterKeyTest { } private static String randomString(int length) { - Random random = new Random(); StringBuilder builder = new StringBuilder(); while (length > 0) {