SpotBugs (4.9.6) Analysis for app (spotbugsGplayDebug)

SpotBugs Analysis generated at: Fri, 26 Sep 2025 03:15:14 +0000

Package Code Size Bugs High Prio Bugs Medium Prio Bugs Low Prio Bugs Exp. Bugs Ratio
Overall (85 packages), (1646 classes) 56678 500 22 478
com.nextcloud.android.sso 370 10 3 7
com.nextcloud.android.sso.aidl 271 7 7
com.nextcloud.client.account 263 7 7
com.nextcloud.client.database.migrations 484 1 1
com.nextcloud.client.network 219 1 1
com.owncloud.android 704 3 3
com.owncloud.android.authentication 1214 9 9
com.owncloud.android.datamodel 4543 49 1 48
com.owncloud.android.datamodel.e2e.v1.encrypted 46 1 1
com.owncloud.android.datastorage 135 1 1
com.owncloud.android.datastorage.providers 145 3 3
com.owncloud.android.db 601 5 5
com.owncloud.android.files 587 5 5
com.owncloud.android.files.services 143 1 1
com.owncloud.android.operations 2949 78 4 74
com.owncloud.android.providers 1230 16 16
com.owncloud.android.services 517 8 1 7
com.owncloud.android.syncadapter 318 4 4
com.owncloud.android.ui 249 3 3
com.owncloud.android.ui.activities.data.activities 103 1 1
com.owncloud.android.ui.activity 6219 56 2 54
com.owncloud.android.ui.adapter 3341 44 44
com.owncloud.android.ui.asynctasks 616 24 24
com.owncloud.android.ui.dialog.parcel 198 1 1
com.owncloud.android.ui.fragment 3718 65 1 64
com.owncloud.android.ui.fragment.contactsbackup 400 10 10
com.owncloud.android.ui.helpers 668 10 10
com.owncloud.android.ui.preview 774 19 1 18
com.owncloud.android.utils 3327 54 8 46
com.owncloud.android.utils.theme 159 4 4
PSC / PSC_PRESIZE_COLLECTIONS

This method allocates a collection using the default constructor even though it is known a priori (or at least can be reasonably guessed) how many items are going to be placed in the collection, and thus needlessly causes intermediate reallocations of the collection.

You can use the constructor that takes an initial size and that will be much better, but due to the loadFactor of Maps and Sets, even this will not be a correct estimate.

If you are using Guava, use its methods that allocate maps and sets with a predetermined size, to get the best chance for no reallocations, such as:

If not, a good estimate would be the expectedSize / {LOADING_FACTOR} which by default is 0.75

CLI / CLI_CONSTANT_LIST_INDEX

This method accesses an array or list using a constant integer index. Often, this is a typo where a loop variable is intended to be used. If however, specific list indices mean different specific things, then perhaps replacing the list with a first-class object with meaningful accessors would make the code less brittle.

WOC / WOC_WRITE_ONLY_COLLECTION_FIELD

This class creates and initializes a collection as a field but then never accesses this collection to gain information or fetch items from the collection. It is likely that this collection is left over from a past effort, and can be removed.

SECHPP / HTTP_PARAMETER_POLLUTION

Concatenating unvalidated user input into a URL can allow an attacker to override the value of a request parameter. Attacker may be able to override existing parameter values, inject a new parameter or exploit variables out of a direct reach. HTTP Parameter Pollution (HPP) attacks consist of injecting encoded query string delimiters into other existing parameters. If a web application does not properly sanitize the user input, a malicious user may compromise the logic of the application to perform either client-side or server-side attacks.
In the following example the programmer has not considered the possibility that an attacker could provide a parameter lang such as en&user_id=1, which would enable him to change the user_id at will.

Vulnerable Code:

String input = request.getParameter("lang");
GetMethod get = new GetMethod("http://www.host.com/viewDetails");
get.setQueryString("lang=" + input + "&user_id=" + userId);
get.execute();

Solution:
You can either encode user input before placing it in HTTP parameters or use the UriBuilder class from Apache HttpClient.

URIBuilder uriBuilder = new URIBuilder("http://www.host.com/viewDetails");
uriBuilder.addParameter("lang", input);
uriBuilder.addParameter("user_id", userId);

HttpGet httpget = new HttpGet(uriBuilder.build().toString()); //OK


References
CAPEC-460: HTTP Parameter Pollution (HPP) CWE-235: Improper Handling of Extra Parameters

REC / REC_CATCH_EXCEPTION

This method uses a try-catch block that catches Exception objects, but Exception is not thrown within the try block, and RuntimeException is not explicitly caught. It is a common bug pattern to say try { ... } catch (Exception e) { something } as a shorthand for catching a number of types of exception each of whose catch blocks is identical, but this construct also accidentally catches RuntimeException as well, masking potential bugs.

A better approach is to either explicitly catch the specific exceptions that are thrown, or to explicitly catch RuntimeException exception, rethrow it, and then catch all non-Runtime Exceptions, as shown below:

try {
    ...
} catch (RuntimeException e) {
    throw e;
} catch (Exception e) {
    ... deal with all non-runtime exceptions ...
}

See CWE-396: Declaration of Catch for Generic Exception.

SCII / SCII_SPOILED_CHILD_INTERFACE_IMPLEMENTOR

This class declares that it implements an interface, but does so by relying on methods supplied by superclasses, even though those superclasses know nothing about the interface in question. If you wish to have the child not implement all the methods of the interface, it would probably be better to declare the superclass as implementing the interface, and if that class does not provide all the methods, then declare that superclass abstract.

RCN / RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE

This method contains a redundant check of a known non-null value against the constant null.

NAB / NAB_NEEDLESS_BOOLEAN_CONSTANT_CONVERSION

This method assigns a Boxed boolean constant to a primitive boolean variable, or assigns a primitive boolean constant to a Boxed boolean variable. Use the correct constant for the variable desired. Use


boolean b = true;
boolean b = false;
or

Boolean b = Boolean.TRUE;
Boolean b = Boolean.FALSE;

Be aware that this boxing happens automatically when you might not expect it. For example,


Map statusMap = ...

public Boolean someMethod() {
    statusMap.put("foo", true);  //the "true" here is boxed
    return false;  //the "false" here is boxed
}
has two cases of this needless autoboxing. This can be made more efficient by simply substituting in the constant values:

Map statusMap = ...

public Boolean someMethod() {
    statusMap.put("foo", Boolean.TRUE);
    return Boolean.FALSE;
}

PL / PL_PARALLEL_LISTS

This class appears to maintain two or more lists or arrays whose contents are related in a parallel way. That is, you have something like:


List<String> words = new ArrayList<String>();
List<Integer> wordCounts = new ArrayList<String>();
where the elements of the list at index 0 are related, the elements at index 1 are related and so on.

Consider creating a separate class to hold all the related pieces of information, and adding instances of this class to just one list or array, or if just two values, use a Map to associate one value with the other like:


private class WordAndCount{public String word;  public int count}

List<WordAndCount> wordsAndCounts = new ArrayList<WordAndCount>();
//or, for just two elements
Map wordCounts = new HashMap();

OCP / OCP_OVERLY_CONCRETE_PARAMETER

This method uses concrete classes for parameters when only methods defined in an implemented interface or superclass are used. Consider increasing the abstraction of the interface to make low impact changes easier to accomplish in the future.

Take the following example:


private void appendToList(ArrayList<String> list) {
    if (list.size() < 100) {
        list.add("Foo");
    }
}
The parameter list is currently defined as an ArrayList, which is a concrete implementation of the List interface. Specifying ArrayList is unnecessary here, because we aren't using any ArrayList-specific methods (like ensureCapacity() or trimToSize()). Instead of using the concrete definition, it is better to do something like:

private void appendToList(List<String> list) {
    ...
If the design ever changes, e.g. a LinkedList is used instead, this code won't have to change.

IDEs tend to have tools to help generalize parameters. For example, in Eclipse, the refactoring tool Generalize Declared Type helps find an appropriate level of concreteness.

MOM / MOM_MISLEADING_OVERLOAD_MODEL

This class 'overloads' the same method with both instance and static versions. As the use of these two models is different, it will be confusing to the users of these methods.

CFS / CFS_CONFUSING_FUNCTION_SEMANTICS

This method appears to modify a parameter, and then return this parameter as the method's return value. This will be confusing to callers of this method, as it won't be apparent that the 'original' passed-in parameter will be changed as well. If the purpose of this method is to change the parameter, it would be more clear to change the method to have a void return value. If a return type is required due to interface or superclass contract, perhaps a clone of the parameter should be made.

SACM / SACM_STATIC_ARRAY_CREATED_IN_METHOD

This method creates an array initialized by constants. Each time this method is called this array will be recreated. It would be more performant to define the array as a static field of the class instead.

Se / SE_COMPARATOR_SHOULD_BE_SERIALIZABLE

This class implements the Comparator interface. You should consider whether or not it should also implement the Serializable interface. If a comparator is used to construct an ordered collection such as a TreeMap, then the TreeMap will be serializable only if the comparator is also serializable. As most comparators have little or no state, making them serializable is generally easy and good defensive programming.

See CWE-1066: Missing Serialization Control Element.

UCF / UCF_USELESS_CONTROL_FLOW

This method contains a useless control flow statement, where control flow continues onto the same place regardless of whether or not the branch is taken. For example, this is caused by having an empty statement block for an if statement:

if (argv.length == 0) {
    // TODO: handle this case
}
DRE / DRE_DECLARED_RUNTIME_EXCEPTION

This method declares a RuntimeException derived class in its throws clause. This may indicate a misunderstanding as to how unchecked exceptions are handled. If it is felt that a RuntimeException is so prevalent that it should be declared for documentation purposes, using javadoc's @throws clause is a better idea, as it doesn't needless pollute the method signature,

SECBROAD / ANDROID_BROADCAST

Broadcast intents can be listened by any application with the appropriate permission. It is suggested to avoid transmitting sensitive information when possible.

Code at risk:

Intent i = new Intent();
i.setAction("com.insecure.action.UserConnected");
i.putExtra("username", user);
i.putExtra("email", email);
i.putExtra("session", newSessionId);

this.sendBroadcast(v1);


Solution (if possible):

Intent i = new Intent();
i.setAction("com.secure.action.UserConnected");

sendBroadcast(v1);


Configuration (receiver)[1] Source: StackOverflow:

<manifest ...>

    <!-- Permission declaration -->
    <permission android:name="my.app.PERMISSION" />

    <receiver
        android:name="my.app.BroadcastReceiver"
        android:permission="my.app.PERMISSION"> <!-- Permission enforcement -->
        <intent-filter>
            <action android:name="com.secure.action.UserConnected" />
        </intent-filter>
    </receiver>

    ...
</manifest>

Configuration (sender)[1] Source: StackOverflow:

<manifest>
    <!-- We declare we own the permission to send broadcast to the above receiver -->
    <uses-permission android:name="my.app.PERMISSION"/>

    <!-- With the following configuration, both the sender and the receiver apps need to be signed by the same developer certificate. -->
    <permission android:name="my.app.PERMISSION" android:protectionLevel="signature"/>
</manifest>


References
CERT: DRD03-J. Do not broadcast sensitive information using an implicit intent
Android Official Doc: BroadcastReceiver (Security)
Android Official Doc: Receiver configuration (see android:permission)
[1] StackOverflow: How to set permissions in broadcast sender and receiver in android
CWE-276: Incorrect Default Permissions
CWE-925: Improper Verification of Intent by Broadcast Receiver
CWE-927: Use of Implicit Intent for Sensitive Communication

PCOA / PCOA_PARTIALLY_CONSTRUCTED_OBJECT_ACCESS

This constructor makes a call to a non-final method. Since this method can be overridden, a subclass' implementation will be executing against an object that has not been initialized at the subclass level. You should mark all methods called from the constructor as final to avoid this problem.

SEC / SEC_SIDE_EFFECT_CONSTRUCTOR

This method creates an object but does not assign this object to any variable or field. This implies that the class operates through side effects in the constructor, which is a bad pattern to use, as it adds unnecessary coupling. Consider pulling the side effect out of the constructor, into a separate method, or into the calling method.

ISB / ISB_TOSTRING_APPENDING

This method concatenates the output of a toString() call into a StringBuffer or StringBuilder. It is simpler just to pass the object you want to append to the append call, as that form does not suffer the potential for NullPointerExceptions, and is easier to read.

Keep in mind that Java compiles simple String concatenation to use StringBuilders, so you may see this bug even when you don't use StringBuilders explicitly.

Instead of:


StringBuilder builder = ...;
builder.append(someObj.toString());
...
System.out.println("Problem with the object :" + someObj.toString());
just do:

StringBuilder builder = ...
builder.append(someObj);
...
System.out.println("Problem with the object :" + someObj);
to avoid the possibility of NullPointerExceptions when someObj is null.

STT / STT_STRING_PARSING_A_FIELD

This method calls a parsing method (indexOf, lastIndexOf, startsWith, endsWith, substring, indexOf) on a String that is a field, or comes from a collection that is a field. This implies that the String in question is holding multiple parts of information inside the string, which would be more maintainable and type safe if that value was a true collection or a first class object with fields, rather than a String.

UCPM / UCPM_USE_CHARACTER_PARAMETERIZED_METHOD

This method passes a constant literal String of length 1 as a parameter to a method, when a similar method is exposed that takes a char. It is simpler and more expedient to handle one character, rather than a String.

Instead of making calls like:


String myString = ...
if (myString.indexOf("e") != -1) {
    int i = myString.lastIndexOf("e");
    System.out.println(myString + ":" + i);  //the Java compiler will use a StringBuilder internally here [builder.append(":")]
    ...
    return myString.replace("m","z");
}
Replace the single letter Strings with their char equivalents like so:

String myString = ...
if (myString.indexOf('e') != -1) {
    int i = myString.lastIndexOf('e');
    System.out.println(myString + ':' + i);  //the Java compiler will use a StringBuilder internally here [builder.append(':')]
    ...
    return myString.replace('m','z');
}

AT / AT_NONATOMIC_64BIT_PRIMITIVE

The long and the double are 64-bit primitive types, and depending on the Java Virtual Machine implementation assigning a value to them can be treated as two separate 32-bit writes, and as such it's not atomic. See JSL 17.7. Non-Atomic Treatment of double and long. See SEI CERT rule VNA05-J. Ensure atomicity when reading and writing 64-bit values and CWE-667: Improper Locking for more info. This bug can be ignored in platforms which guarantee that 64-bit long and double type read and write operations are atomic.

To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from java.lang.concurrent.atomic or correctly synchronize the code. Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations.

EXS / EXS_EXCEPTION_SOFTENING_RETURN_FALSE

This method catches an exception and returns a boolean that represents whether an exception occurred or not. This throws away the value of exception handling and lets code ignore the resultant 'error code' return value. You should just throw the exception to the caller instead.

MUI / MUI_CONTAINSKEY_BEFORE_GET

This method checks for the presence of a key in a map using containsKey(), before attempting to fetch the value of the key using get(). This equates to doing two map lookups in a row. It is much simpler to just fetch the value with get, and checking for non null instead.

As an example, instead of using

    	    	Map myMap = getSomeMap();
    	    	if (myMap.containsKey("foo")) {
    	    		String value = myMap.get("foo");
    	    		....
    	    	}
    	    
convert this to
     	    	Map myMap = getSomeMap();
     	    	String value = myMap.get("foo");
    	    	if (value != null) {
    	    		....
    	    	}
    	    

The only caveat to this is that if you use a null value in a map to represent a third state for the key, then in this case using containsKey is 'correct'. This means an entry found in the map with a null value is taken differently than no entry at all. However, this is a very subtle programming paradigm, and likely to cause problems. If you wish to mark an entry as not being present, it is better to use a named 'sentinel' value to denote this, so instead of:

    	    	Map myMap = getSomeMap();
    	    	if (myMap.containsKey("foo")) {
    	    		String value = myMap.get("foo");
    	    		....
    	    	}
    	    
convert this to
     	    	Map myMap = getSomeMap();
     	    	String value = myMap.get("foo");
    	    	if (NOT_FOUND.equals(value)) {
    	    		....
    	    	}
    	    	where NOT_FOUND is some constant that denotes this special status. Of course you will need to find a special
    	    	sentinel value for each type you are using that isn't possible to have normally.
    	    

RV / RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT

This code calls a method and ignores the return value. However, our analysis shows that the method (including its implementations in subclasses if any) does not produce any effect other than return value. Thus, this call can be removed.

We are trying to reduce the false positives as much as possible, but in some cases this warning might be wrong. Common false-positive cases include:

- The method is designed to be overridden and produce a side effect in other projects which are out of the scope of the analysis.

- The method is called to trigger the class loading which may have a side effect.

- The method is called just to get some exception.

If you feel that our assumption is incorrect, you can use a @CheckReturnValue annotation to instruct SpotBugs that ignoring the return value of this method is acceptable.

SPP / SPP_FIELD_COULD_BE_STATIC

This instance field appears to only be set to a literal value in a constructor, and never reset. Thus it is likely that this field could just be defined as a static field instead.

NP / NP_GUARANTEED_DEREF

There is a statement or branch that if executed guarantees that a value is null at this point, and that value that is guaranteed to be dereferenced (except on forward paths involving runtime exceptions).

Note that a check such as if (x == null) throw new NullPointerException(); is treated as a dereference of x.

EXS / EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS

This method is not constrained by an interface or superclass, but converts a caught checked exception to an unchecked exception and throws it. It would be more appropriate just to throw the checked exception, adding the exception to the throws clause of the method.

MDM / MDM_STRING_BYTES_ENCODING

The behavior of the String(byte[] bytes) and String.getBytes() is undefined if the string cannot be encoded in the platform's default charset. Instead, use the String(byte[] bytes, String encoding) or String.getBytes(String encoding) constructor which accepts the string's encoding as an argument. Be sure to specify the encoding used for the user's locale.

As per the Java specifications, "UTF-8", "US-ASCII", "UTF-16" and "ISO-8859-1" will all be valid encoding charsets. If you aren't sure, try "UTF-8".

New in Java 1.7, you can specify an encoding from StandardCharsets, like StandardCharsets.UTF_8. These are generally preferrable because you don't have to deal with UnsupportedEncodingException.

SUA / SUA_SUSPICIOUS_UNINITIALIZED_ARRAY

This method returns an array that was allocated but apparently not initialized. It is possible that the caller of this method will do the work of initializing this array, but that is not a common pattern, and it is assumed that this array has just been forgotten to be initialized.

Eq / EQ_COMPARETO_USE_OBJECT_EQUALS

This class defines a compareTo(...) method but inherits its equals() method from java.lang.Object. Generally, the value of compareTo should return zero if and only if equals returns true. If this is violated, weird and unpredictable failures will occur in classes such as PriorityQueue. In Java 5 the PriorityQueue.remove method uses the compareTo method, while in Java 6 it uses the equals method.

From the JavaDoc for the compareTo method in the Comparable interface:

It is strongly recommended, but not strictly required that (x.compareTo(y)==0) == (x.equals(y)). Generally speaking, any class that implements the Comparable interface and violates this condition should clearly indicate this fact. The recommended language is "Note: this class has a natural ordering that is inconsistent with equals."

CC / CC_CYCLOMATIC_COMPLEXITY

This method has a high cyclomatic complexity figure, which represents the number of branch points. It is likely difficult to test, and is brittle to change. Consider refactoring this method into several to reduce the risk.

ITU / ITU_INAPPROPRIATE_TOSTRING_USE

This method calls algorithmic operations on a String that was returned from a toString() method. As these methods are for debugging/logging purposes, it shouldn't be the basis of core logic in your code.

IOI / IOI_USE_OF_FILE_STREAM_CONSTRUCTORS

This method creates and uses a java.io.FileInputStream or java.io.FileOutputStream object. Unfortunately both of these classes implement a finalize method, which means that objects created will likely hang around until a full garbage collection occurs, which will leave excessive garbage on the heap for longer, and potentially much longer than expected. Java 7 introduced two ways to create streams for reading and writing files that do not have this concern. You should consider switching from these above classes to InputStream is = java.nio.file.Files.newInputStream(myfile.toPath()); OutputStream os = java.nio.file.Files.newOutputStream(myfile.toPath());

HES / HES_EXECUTOR_NEVER_SHUTDOWN

Most ExecutorService objects must be explicitly shut down, otherwise their internal threads can prolong the running of the JVM, even when everything else has stopped.

FindBugs has detected that there are no calls to either the shutdown() or shutdownNow() method, and thus, the ExecutorService is not guaranteed to ever terminate. This is especially problematic for Executors.newFixedThreadPool() and most of the other convenience methods in the Executors class.

Even though there are some exceptions to this, particularly when a custom ThreadFactory is provided, or for ThreadPoolExecutors with allowsCoreThreadTimeOut() set to true, it is good practice to explicitly shutdown the ExecutorService when its utility is done.

SING / SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR

This class is using singleton design pattern and has non-private constructor (please note that a default constructor might exist which is not private). Given that, it is possible to create a copy of the object, thus violating the singleton pattern.
The easier solution would be making the constructor private.

For more information, see: SEI CERT MSC07-J, and CWE-543: Use of Singleton Pattern Without Synchronization in a Multithreaded Context.

MS / MS_CANNOT_BE_FINAL

A mutable static field could be changed by malicious code or by accident from another package. Unfortunately, the way the field is used doesn't allow any easy fix to this problem.

Dm / DM_DEFAULT_ENCODING

Found a call to a method which will perform a byte to String (or String to byte) conversion, and will assume that the default platform encoding is suitable. This will cause the application behavior to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

See CWE-173: Improper Handling of Alternate Encoding.

FE / FE_FLOATING_POINT_EQUALITY

This operation compares two floating point values for equality. Because floating point calculations may involve rounding, calculated float and double values may not be accurate. For values that must be precise, such as monetary values, consider using a fixed-precision type such as BigDecimal. For values that need not be precise, consider comparing for equality within some range, for example: if ( Math.abs(x - y) < .0000001 ). See the Java Language Specification, section 4.2.4.

UrF / URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD

This field is never read.  The field is public or protected, so perhaps it is intended to be used with classes not seen as part of the analysis. If not, consider removing it from the class.

See CWE-563: Assignment to Variable without Use.

OCP / OCP_OVERLY_CONCRETE_COLLECTION_PARAMETER

This method uses specific collection interface/classes for parameters when only methods defined in the Collection class are used. Consider increasing the abstraction of the interface to make low impact changes easier to accomplish in the future.

Take the following example:


private void printList(List<String> list) {
    for (String s : list) {
        System.out.println(s);
    }
}
The parameter list is currently defined as an List, which is a specific collection interface. Specifying List is unnecessary here, because we aren't using any AList-specific methods (like get(0)). Instead of using the concrete definition, it is better to do something like:

private void printList(Collection<String> list) {
    ...
If the design ever changes, e.g. a Set is used instead, this code won't have to change.

IDEs tend to have tools to help generalize parameters. For example, in Eclipse, the refactoring tool Generalize Declared Type helps find an appropriate level of concreteness.

UVA / UVA_REMOVE_NULL_ARG

This method calls a var arg method, and passes an explicit null value as the var arg parameter. It is better to just not pass any value at all, as there is confusion as to whether the null represents the entire vararg array, or the first element of the varargs array

JLM / JLM_JSR166_UTILCONCURRENT_MONITORENTER

This method performs synchronization on an object that is an instance of a class from the java.util.concurrent package (or its subclasses). Instances of these classes have their own concurrency control mechanisms that are orthogonal to the synchronization provided by the Java keyword synchronized. For example, synchronizing on an AtomicBoolean will not prevent other threads from modifying the AtomicBoolean.

Such code may be correct, but should be carefully reviewed and documented, and may confuse people who have to maintain the code at a later date.

DCN / DCN_NULLPOINTER_EXCEPTION

According to SEI Cert rule ERR08-J NullPointerException should not be caught. Handling NullPointerException is considered an inferior alternative to null-checking.

This non-compliant code catches a NullPointerException to see if an incoming parameter is null:


boolean hasSpace(String m) {
  try {
    String ms[] = m.split(" ");
    return names.length != 1;
  } catch (NullPointerException e) {
    return false;
  }
}

A compliant solution would use a null-check as in the following example:


boolean hasSpace(String m) {
    if (m == null) return false;
    String ms[] = m.split(" ");
    return names.length != 1;
}

See CWE-395: Use of NullPointerException Catch to Detect NULL Pointer Dereference.

PRMC / PRMC_POSSIBLY_REDUNDANT_METHOD_CALLS

This method makes two consecutive calls to the same method, using the same constant parameters, on the same instance, without any intervening changes to the objects. If this method does not make changes to the object, which it appears it doesn't, then making two calls is just a waste. These method calls could be combined by assigning the result into a temporary variable, and using the variable the second time.

SECCI / COMMAND_INJECTION

The highlighted API is used to execute a system command. If unfiltered input is passed to this API, it can lead to arbitrary command execution.


Vulnerable Code:

import java.lang.Runtime;

Runtime r = Runtime.getRuntime();
r.exec("/bin/sh -c some_tool" + input);

References
OWASP: Command Injection
OWASP: Top 10 2013-A1-Injection
CWE-78: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection')

EXS / EXS_EXCEPTION_SOFTENING_NO_CHECKED

This method's exception signature is constrained by an interface or superclass not to throw any checked exceptions. Therefore a caught checked exception was converted to an unchecked exception and thrown. However, it appears that the class in question is owned by the same author as the constraining interface or superclass. Consider changing the signature of this method to include the checked exception.

RV / RV_RETURN_VALUE_IGNORED_BAD_PRACTICE

This method returns a value that is not checked. The return value should be checked since it can indicate an unusual or unexpected function execution. For example, the File.delete() method returns false if the file could not be successfully deleted (rather than throwing an Exception). If you don't check the result, you won't notice if the method invocation signals unexpected behavior by returning an atypical return value.

See CWE-253: Incorrect Check of Function Return Value.

OS / OS_OPEN_STREAM

The method creates an IO stream object, does not assign it to any fields, pass it to other methods that might close it, or return it, and does not appear to close the stream on all paths out of the method.  This may result in a file descriptor leak.  It is generally a good idea to use a finally block to ensure that streams are closed.

See CWE-459: Incomplete Cleanup.

PMB / PMB_POSSIBLE_MEMORY_BLOAT

This class defines static fields that are Collections, StringBuffers, or StringBuilders that do not appear to have any way to clear or reduce their size. That is, a collection is defined and has method calls like
{add(), append(), offer(), put(), ...}
with no method calls to removal methods like
{clear(), delete(), pop(), remove(), ...}
This means that the collection in question can only ever increase in size, which is a potential cause of memory bloat.

If this collection is a list, set or otherwise of static things (e.g. a List<String> for month names), consider adding all of the elements in a static initializer, which can only be called once:


private static List<String> monthNames = new ArrayList<String>();
static {
    monthNames.add("January");
    monthNames.add("February");
    monthNames.add("March");
    ...
}

SIC / SIC_INNER_SHOULD_BE_STATIC

This class is an inner class, but does not use its embedded reference to the object which created it.  This reference makes the instances of the class larger, and may keep the reference to the creator object alive longer than necessary.  If possible, the class should be made static.

OC / OC_OVERZEALOUS_CASTING

This method casts the right hand side of an expression to a class that is more specific than the variable on the left hand side of the assignment. The cast only has to be as specific as the variable that is on the left. Using a more specific type on the right hand side just increases cohesion.

NP / NP_NULL_ON_SOME_PATH_FROM_RETURN_VALUE

The return value from a method is dereferenced without a null check, and the return value of that method is one that should generally be checked for null. This may lead to a NullPointerException when the code is executed.

LII / LII_LIST_INDEXED_ITERATING

This method uses an integer-based for loop to iterate over a java.util.List, by calling List.get(i) each time through the loop. The integer is not used for other reasons. It is better to use an Iterator instead, as depending on List implementation, iterators can perform better, and they also allow for exchanging of other collection types without issue.

ENMI / ENMI_NULL_ENUM_VALUE

This method sets the value of an enum reference to null. An enum should never have a null value. If there is a state where you do not know what the value of an enum should be, than that should be one of the proper enum values. So add a MyEnum.UNKNOWN or such. This keeps the logic of switch statements, etc, much simpler.

ST / ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD

This instance method writes to a static field. This is tricky to get correct if multiple instances are being manipulated, and generally bad practice.

AFBR / AFBR_ABNORMAL_FINALLY_BLOCK_RETURN

This method returns or throws exceptions from a finally block. This will mask real program logic in the try block, and short-circuit normal method termination.

SF / SF_SWITCH_NO_DEFAULT

This method contains a switch statement where default case is missing. Usually you need to provide a default case.

Because the analysis only looks at the generated bytecode, this warning can be incorrect triggered if the default case is at the end of the switch statement and the switch statement doesn't contain break statements for other cases.

See CWE-478: Missing Default Case in Multiple Condition Expression.

BC / BC_VACUOUS_INSTANCEOF

This instanceof test will always return true (unless the value being tested is null). Although this is safe, make sure it isn't an indication of some misunderstanding or some other logic error. If you really want to test the value for being null, perhaps it would be clearer to do better to do a null test rather than an instanceof test.

See CWE-571: Expression is Always True.

SEO / SEO_SUBOPTIMAL_EXPRESSION_ORDER

This method builds a conditional expression, for example, in an if or while statement, where the expressions contain both simple local variable comparisons and comparisons on method calls. The expression orders these so that the method calls come before the simple local variable comparisons. This causes method calls to be executed in conditions when they do not need to be, and thus potentially causes a lot of code to be executed for nothing. By ordering the expressions so that the simple conditions containing local variable conditions are first, you eliminate this waste. This assumes that the method calls do not have side effects. If the methods do have side effects, it is probably a better idea to pull these calls out of the condition and execute them first, assigning a value to a local variable. In this way you give a hint that the call may have side effects.

Example:


if ((calculateHaltingProbability() > 0) && shouldCalcHalting) { }
would be better as

if (shouldCalcHalting && (calculateHaltingProbability() > 0) { }

IMC / IMC_IMMATURE_CLASS_COLLECTION_RETURN

This method declares that it returns a Collection, when in fact it has created and returned a more specific interface. Since the caller is free to cast any return value to Collection, it is better to return the specific interface created so that it gives the caller the most power. With methods, always expect the least with parameters, and provide the most with return types.

UTWR / UTWR_USE_TRY_WITH_RESOURCES
[

This method allocates and uses an auto closeable resource. However, it manually closes the resource in a finally block. While this is correct management, it doesn't rely on the idiomatic way available to JDK 7 and above, allows for possible subtle problems, and complicates the reading of code by developers expecting the use of try-with-resources.

Switch to using try with resources, as:

    		    try (InputStream is = getAStream()) {
    		        useTheStream(is);
    		    }
    		
SBSC / SBSC_USE_STRINGBUFFER_CONCATENATION

The method seems to be building a String using concatenation in a loop. In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.

Better performance can be obtained by using a StringBuffer (or StringBuilder in Java 5) explicitly.

For example:

// This is bad
String s = "";
for (int i = 0; i < field.length; ++i) {
    s = s + field[i];
}

// This is better
StringBuffer buf = new StringBuffer();
for (int i = 0; i < field.length; ++i) {
    buf.append(field[i]);
}
String s = buf.toString();
SGSU / SGSU_SUSPICIOUS_GETTER_SETTER_USE

This method retrieves the property of a Java bean, only to use it in the setter for the same property of the same bean. This is usually a copy/paste typo.

SECUHE / UNSAFE_HASH_EQUALS

An attacker might be able to detect the value of the secret hash due to the exposure of comparison timing. When the functions Arrays.equals() or String.equals() are called, they will exit earlier if fewer bytes are matched.

Vulnerable Code:

String actualHash = ...

if(userInput.equals(actualHash)) {
    ...
}

Solution:

String actualHash = ...

if(MessageDigest.isEqual(userInput.getBytes(),actualHash.getBytes())) {
    ...
}


References
CWE-203: Information Exposure Through DiscrepancyKey

SECOBDES / OBJECT_DESERIALIZATION

Object deserialization of untrusted data can lead to remote code execution, if there is a class in classpath that allows the trigger of malicious operation.

Libraries developers tend to fix class that provided potential malicious trigger. There are still classes that are known to trigger Denial of Service[1].

Deserialization is a sensible operation that has a great history of vulnerabilities. The web application might become vulnerable as soon as a new vulnerability is found in the Java Virtual Machine[2] [3].

Code at risk:

public UserData deserializeObject(InputStream receivedFile) throws IOException, ClassNotFoundException {

    try (ObjectInputStream in = new ObjectInputStream(receivedFile)) {
        return (UserData) in.readObject();
    }
}

Solutions:

Avoid deserializing object provided by remote users.


References
CWE-502: Deserialization of Untrusted Data
Deserialization of untrusted data
Serialization and Deserialization
A tool for generating payloads that exploit unsafe Java object deserialization
[1] Example of Denial of Service using the class java.util.HashSet
[2] OpenJDK: Deserialization issue in ObjectInputStream.readSerialData() (CVE-2015-2590)
[3] Rapid7: Sun Java Calendar Deserialization Privilege Escalation (CVE-2008-5353)

NP / NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE

This parameter is always used in a way that requires it to be non-null, but the parameter is explicitly annotated as being Nullable. Either the use of the parameter or the annotation is wrong.

BL / BL_BURYING_LOGIC

Looks for relatively large if blocks of code, where you unconditionally return from them, and then follow that with an unconditional return of a small block. This places the bulk of the logic to the right indentation-wise, making it more difficult to read than needed. It would be better to invert the logic of the if block, and immediately return, allowing the bulk of the logic to be move to the left for easier reading.

US / US_USELESS_SUPPRESSION_ON_METHOD

Suppressing annotations &SuppressFBWarnings should be removed from the source code as soon as they are no more needed. Leaving them may result in accidental warnings suppression. The annotation was probably added to suppress a warning raised by SpotBugs, but now SpotBugs does not report the bug anymore. Either the bug was solved or SpotBugs was updated, and the bug is no longer raised by that code. The unnecessary annotation is not reported when the class or method is annotated with an &Generated annotation with retention CLASS or RUNTIME.

AT / AT_STALE_THREAD_WRITE_OF_PRIMITIVE

SEI CERT rule VNA00-J describes that reading a shared primitive variable in one thread may not yield the value of the most recent write to the variable from another thread. Consequently, the thread may observe a stale value of the shared variable.

To fix it, declare the variable volatile, change the type of the field to the corresponding atomic type from java.lang.concurrent.atomic or correctly synchronize the code. Declaring the variable volatile may not be enough in some cases: e.g. when the variable is assigned a value which depends on the current value or on the result of nonatomic compound operations. This guarantees that 64-bit primitive long and double variables are accessed atomically.

See also CWE-413: Improper Resource Locking, CWE-567: Unsynchronized Access to Shared Data in a Multithreaded Context, and CWE-667: Improper Locking.

US / US_USELESS_SUPPRESSION_ON_CLASS

Suppressing annotations &SuppressFBWarnings should be removed from the source code as soon as they are no more needed. Leaving them may result in accidental warnings suppression. The unnecessary annotation is not reported when the class is annotated with an &Generated annotation with retention CLASS or RUNTIME.

UAC / UAC_UNNECESSARY_API_CONVERSION_FILE_TO_PATH

This method creates a java.nio.file.Path object by first creating a java.io.File object, and then calling toPath() on it. It is simpler to just construct the Path object directly, say by using {@code Paths.get(String...)}.

NAB / NAB_NEEDLESS_BOXING_VALUEOF

This method passes a String to a wrapped primitive object's parse method, which in turn calls the valueOf method to convert to a boxed primitive. When it is desired to convert from a String to a boxed primitive object, it is simpler to use the BoxedPrimitive.valueOf(String) method.

Instead of something like:


Boolean bo = Boolean.valueOf(Boolean.parseBoolean("true"));
Float f = Float.valueOf(Float.parseFloat("1.234"));
Simply do:

Boolean bo = Boolean.valueOf("true");
Float f = Float.valueOf("1.234");

SECXML / POTENTIAL_XML_INJECTION

Unsafe strings (e.g. from a user input) may contain XML tags. If such a string is inserted into an XML document then it may change its structure to a valid but semantically different document. To prevent this unsafe strings must b sanitized first.

Example:
An XML document may look like this:

⟨product⟩
    ⟨name⟩Cell Phone⟨/name⟩
    ⟨price⟩800⟨/price⟩
    ⟨amount⟩1⟨/amount⟩
⟨/product⟩

An attacker may put the following string to the field "amount":
1⟨/amount⟩⟨price⟩1⟨/price⟩⟨amount⟩1

If the XML parser works in a way that the second price overwrites the first one then the attacker may buy the cell phone for 1 singe dollar.

References
IDS16-J. Prevent XML Injection

FCBL / FCBL_FIELD_COULD_BE_LOCAL

This class defines fields that are used in a local only fashion, specifically private fields or protected fields in final classes that are accessed first in each method with a store vs. a load. This field could be replaced by one or more local variables.

RFI / RFI_SET_ACCESSIBLE

This method uses the reflective setAccessible method to alter the behavior of methods and fields in classes in ways that were not expected to be accessed by the author. Doing so circumvents the protections that the author provided through the class definition, and may expose your application to unexpected side effects and problems. This functionality is deprecated in Java 9, and in Java 10 it is expected that this functionality won't work at all.

RCN / RCN_REDUNDANT_NULLCHECK_OF_NULL_VALUE

This method contains a redundant check of a known null value against the constant null.

SECPTO / PATH_TRAVERSAL_OUT

A file is opened to write to its contents. The filename comes from an input parameter. If an unfiltered parameter is passed to this file API, files at an arbitrary filesystem location could be modified.

This rule identifies potential 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.


References
WASC-33: Path Traversal
OWASP: Path Traversal
CAPEC-126: Path Traversal
CWE-22: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')

SPP / SPP_PASSING_THIS_AS_PARM

This method calls an instance method passing the object that the method is called on as a parameter, such as foo.someMethod(foo); As you already have access to this object thru this, you don't need to pass it.

SPP / SPP_NULL_BEFORE_INSTANCEOF

This method checks a reference for null just before seeing if the reference is an instanceof some class. Since instanceof will return false for null references, the null check is not needed.

CE / CE_CLASS_ENVY

This method makes extensive use of methods from another class over methods of its own class. Typically this means that the functionality that is accomplished by this method most likely belongs with the class that is being used so liberally. Consider refactoring this method to be contained in that class, and to accept all the parameters needed in the method signature.

DMI / DMI_RANDOM_USED_ONLY_ONCE

This code creates a java.util.Random object, uses it to generate one random number, and then discards the Random object. This produces mediocre quality random numbers and is inefficient. If possible, rewrite the code so that the Random object is created once and saved, and each time a new random number is required invoke a method on the existing Random object to obtain it.

If it is important that the generated Random numbers not be guessable, you must not create a new Random for each random number; the values are too easily guessable. You should strongly consider using a java.security.SecureRandom instead (and avoid allocating a new SecureRandom for each random number needed).

AT / AT_NONATOMIC_OPERATIONS_ON_SHARED_VARIABLE

This write of a variable shared between functions depends on the current value of the variable (either because it's a compound operation - e.g. +=, ++ - or it simply depends on the current value), as such it consists of more than one discrete operation. These operations are not atomic in themselves and need further synchronization. See SEI CERT rule VNA02-J, CWE-366: Race Condition within a Thread, CWE-413: Improper Resource Locking, CWE-567: Unsynchronized Access to Shared Data in a Multithreaded Context, and CWE-667: Improper Locking.

Simply declaring a variable volatile fails to guarantee the atomicity of compound operations on the variable, but synchronizing the writes on top of declaring the variable volatile for read operations is sufficient.

To solve this issue, synchronize compound operations and other write operations depending on the previous value, use read-write locks, or declare the shared variable with an atomic type.

SECMD5 / WEAK_MESSAGE_DIGEST_MD5

The algorithms MD2, MD4 and MD5 are not a recommended MessageDigest. PBKDF2 should be used to hash password for example.

"The security of the MD5 hash function is severely compromised. A collision attack exists that can find collisions within seconds on a computer with a 2.6 GHz Pentium 4 processor (complexity of 224.1).[1] Further, there is also a chosen-prefix collision attack that can produce a collision for two inputs with specified prefixes within hours, using off-the-shelf computing hardware (complexity 239).[2]"
- Wikipedia: MD5 - Security
"SHA-224, SHA-256, SHA-384, SHA-512, SHA-512/224, and SHA-512/256:
The use of these hash functions is acceptable for all hash function applications."
- NIST: Transitioning the Use of Cryptographic Algorithms and Key Lengths p.15
"The main idea of a PBKDF is to slow dictionary or brute force attacks on the passwords by increasing the time needed to test each password. An attacker with a list of likely passwords can evaluate the PBKDF using the known iteration counter and the salt. Since an attacker has to spend a significant amount of computing time for each try, it becomes harder to apply the dictionary or brute force attacks."
- NIST: Recommendation for Password-Based Key Derivation p.12

Vulnerable Code:

MessageDigest md5Digest = MessageDigest.getInstance("MD5");
    md5Digest.update(password.getBytes());
    byte[] hashValue = md5Digest.digest();

byte[] hashValue = DigestUtils.getMd5Digest().digest(password.getBytes());


Solution (Using bouncy castle):

public static byte[] getEncryptedPassword(String password, byte[] salt) throws NoSuchAlgorithmException, InvalidKeySpecException {
    PKCS5S2ParametersGenerator gen = new PKCS5S2ParametersGenerator(new SHA256Digest());
    gen.init(password.getBytes("UTF-8"), salt.getBytes(), 4096);
    return ((KeyParameter) gen.generateDerivedParameters(256)).getKey();
}

Solution (Java 8 and later):
public static byte[] getEncryptedPassword(String password, byte[] salt) throws NoSuchAlgorithmException, InvalidKeySpecException {
    KeySpec spec = new PBEKeySpec(password.toCharArray(), salt, 4096, 256 * 8);
    SecretKeyFactory f = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256");
    return f.generateSecret(spec).getEncoded();
}


References
[1] On Collisions for MD5: Master Thesis by M.M.J. Stevens
[2] Chosen-prefix collisions for MD5 and applications: Paper written by Marc Stevens
Wikipedia: MD5
NIST: Transitioning the Use of Cryptographic Algorithms and Key Lengths
NIST: Recommendation for Password-Based Key Derivation
Stackoverflow: Reliable implementation of PBKDF2-HMAC-SHA256 for Java
CWE-327: Use of a Broken or Risky Cryptographic Algorithm CWE-328: Use of Weak Hash

SECSSSRFUC / URLCONNECTION_SSRF_FD

Server-Side Request Forgery occur when a web server executes a request to a user supplied destination parameter that is not validated. Such vulnerabilities could allow an attacker to access internal services or to launch attacks from your web server.

URLConnection can be used with file:// protocol or other protocols to access local filesystem and potentially other services.

Vulnerable Code:

new URL(String url).openConnection()
new URL(String url).openStream()
new URL(String url).getContent()

Solution/Countermeasures:


References
CWE-918: Server-Side Request Forgery (SSRF)
Understanding Server-Side Request Forgery
CWE-73: External Control of File Name or Path
Abusing jar:// downloads

OBL / OBL_UNSATISFIED_OBLIGATION_EXCEPTION_EDGE

This method may fail to clean up (close, dispose of) a stream, database object, or other resource requiring an explicit cleanup operation.

In general, if a method opens a stream or other resource, the method should use a try/finally block to ensure that the stream or resource is cleaned up before the method returns.

This bug pattern is essentially the same as the OS_OPEN_STREAM and ODR_OPEN_DATABASE_RESOURCE bug patterns, but is based on a different (and hopefully better) static analysis technique. We are interested is getting feedback about the usefulness of this bug pattern. For sending feedback, check:

In particular, the false-positive suppression heuristics for this bug pattern have not been extensively tuned, so reports about false positives are helpful to us.

See Weimer and Necula, Finding and Preventing Run-Time Error Handling Mistakes (PDF), for a description of the analysis technique.

PA / PA_PUBLIC_PRIMITIVE_ATTRIBUTE

SEI CERT rule OBJ01-J requires that accessibility to fields must be limited. Otherwise, the values of the fields may be manipulated from outside the class, which may be unexpected or undesired behaviour. In general, requiring that no fields are allowed to be public is overkill and unrealistic. Even the rule mentions that final fields may be public. Besides final fields, there may be other usages for public fields: some public fields may serve as "flags" that affect the behavior of the class. Such flag fields are expected to be read by the current instance (or the current class, in case of static fields), but written by others. If a field is both written by the methods of the current instance (or the current class, in case of static fields) and from the outside, the code is suspicious. Consider making these fields private and provide appropriate setters, if necessary. Please note that constructors, initializers and finalizers are exceptions, if only they write the field inside the class, the field is not considered as written by the class itself.

See CWE-766: Critical Data Element Declared Public.

ISB / ISB_INEFFICIENT_STRING_BUFFERING

This method uses StringBuffer or StringBuilder's append method to concatenate strings. However, it passes the result of doing a simple String concatenation to one of these append calls, thus removing any performance gains of using the StringBuffer or StringBuilder class.

Java will implicitly use StringBuilders, which can make this hard to detect or fix. For example,


StringBuilder sb = new StringBuilder();
for (Map.Entry e : map.entrySet()) {
    sb.append(e.getKey() + e.getValue());		//bug detected here
}

gets automatically turned into something like:

StringBuilder sb = new StringBuilder();
for (Map.Entry e : map.entrySet()) {
    StringBuilder tempBuilder = new StringBuilder();
    tempBuilder.append(e.getKey());
    tempBuilder.append(e.getValue());
    sb.append(tempBuilder.toString());		//this isn't too efficient
}

which involves a temporary StringBuilder, which is completely unnecessary. To prevent this from happening, simply do:

StringBuilder sb = new StringBuilder();
for (Map.Entry e : map.entrySet()) {
    sb.append(e.getKey());
    sb.append(e.getValue());
}

NP / NP_NULL_ON_SOME_PATH

There is a branch of statement that, if executed, guarantees that a null value will be dereferenced, which would generate a NullPointerException when the code is executed. Of course, the problem might be that the branch or statement is infeasible and that the null pointer exception cannot ever be executed; deciding that is beyond the ability of SpotBugs.

NP / NP_LOAD_OF_KNOWN_NULL_VALUE

The variable referenced at this point is known to be null due to an earlier check against null. Although this is valid, it might be a mistake (perhaps you intended to refer to a different variable, or perhaps the earlier check to see if the variable is null should have been a check to see if it was non-null).

LEST / LEST_LOST_EXCEPTION_STACK_TRACE

This method catches an exception, and throws a different exception, without incorporating the original exception. Doing so hides the original source of the exception, making debugging and fixing these problems difficult. It is better to use the constructor of this new exception that takes an original exception so that this detail can be passed along to the user. If this exception has no constructor that takes an initial cause parameter, use the initCause method to initialize it instead.


catch (IOException e) {
    throw new MySpecialException("Failed to open configuration", e);
}

CT / CT_CONSTRUCTOR_THROW

Classes that throw exceptions in their constructors are vulnerable to Finalizer attacks

A finalizer attack can be prevented, by declaring the class final, using an empty finalizer declared as final, or by a clever use of a private constructor.

See SEI CERT Rule OBJ-11 for more information.

SPP / SPP_USE_ISEMPTY

This method calls the size() method on a collection and compares the result to zero to see if the collection is empty. For better code clarity, it is better to just use col.isEmpty() or !col.isEmpty().

ITC / ITC_INHERITANCE_TYPE_CHECKING

This method uses the instanceof operator in a series of if/else statements to differentiate blocks of code based on type. If these types are related by inheritance, it is cleaner to just define a method in the base class, and use overridden methods in these classes.

UCC / UCC_UNRELATED_COLLECTION_CONTENTS

This method adds unrelated objects to a collection or array, requiring careful and brittle data access to that collection. Create a separate class with the properties needed, and add an instance of this class to the collection or array, if possible.

CU / CU_CLONE_USABILITY_OBJECT_RETURN

This class implements the Cloneable interface but defines its clone method to return an Object. Since most likely users of this method will need to cast it to the real type, this will be more painful than necessary. Just declare the return value to be the type of this class.

IS / IS2_INCONSISTENT_SYNC

The fields of this class appear to be accessed inconsistently with respect to synchronization.  This bug report indicates that the bug pattern detector judged that

A typical bug matching this bug pattern is forgetting to synchronize one of the methods in a class that is intended to be thread-safe.

You can select the nodes labeled "Unsynchronized access" to show the code locations where the detector believed that a field was accessed without synchronization.

Note that there are various sources of inaccuracy in this detector; for example, the detector cannot statically detect all situations in which a lock is held.  Also, even when the detector is accurate in distinguishing locked vs. unlocked accesses, the code in question may still be correct.

See CWE-366: Race Condition within a Thread.