SpotBugs Report

Project Information

Project: lib (spotbugsDebugReport)

SpotBugs version: 3.1.12

Code analyzed:



Metrics

2007 lines of code analyzed, in 68 classes, in 9 packages.

Metric Total Density*
High Priority Warnings 12 5.98
Medium Priority Warnings 70 34.88
Total Warnings 82 40.86

(* Defects per Thousand lines of non-commenting source statements)



Contents

Summary

Warning Type Number
Bad practice Warnings 3
Correctness Warnings 21
Internationalization Warnings 2
Multithreaded correctness Warnings 3
Performance Warnings 5
Security Warnings 5
Dodgy code Warnings 43
Total 82

Warnings

Click on a warning row to see full context information.

Bad practice Warnings

Code Warning
HE com.nextcloud.android.sso.aidl.NextcloudRequest defines equals and uses Object.hashCode()
HE com.nextcloud.android.sso.QueryParam defines equals and uses Object.hashCode()
Nm The method name com.nextcloud.android.sso.helper.Retrofit2Helper.WrapInCall(NextcloudAPI, NextcloudRequest, Type) doesn't start with a lower case letter

Correctness Warnings

Code Warning
CLI Method com.nextcloud.android.sso.aidl.ParcelFileDescriptorUtil.pipeFrom(InputStream, IThreadListener) accesses list or array with constant index
CLI Method com.nextcloud.android.sso.aidl.ParcelFileDescriptorUtil.pipeTo(OutputStream, IThreadListener) accesses list or array with constant index
CLI Method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.invoke(NextcloudAPI, Object[]) accesses list or array with constant index
FCBL Class com.nextcloud.android.sso.api.AidlNetworkRequest$ExceptionResponse defines fields that are used only as locals
IMC Class com.nextcloud.android.sso.aidl.NextcloudRequest defines a computed serialVersionUID that doesn't equate to the calculated value
IMC Class com.nextcloud.android.sso.aidl.NextcloudRequest$Builder defines a computed serialVersionUID that doesn't equate to the calculated value
IMC Class com.nextcloud.android.sso.model.SingleSignOnAccount defines a computed serialVersionUID that doesn't equate to the calculated value
IMC Class com.nextcloud.android.sso.QueryParam defines a computed serialVersionUID that doesn't equate to the calculated value
ISB Method com.nextcloud.android.sso.api.AidlNetworkRequest$1.onServiceConnected(ComponentName, IBinder) concatenates the result of a toString() call
ISB Method com.nextcloud.android.sso.api.AidlNetworkRequest$1.onServiceDisconnected(ComponentName) concatenates the result of a toString() call
ISB Method com.nextcloud.android.sso.api.AidlNetworkRequest$1.onServiceDisconnected(ComponentName) concatenates the result of a toString() call
ISB Method com.nextcloud.android.sso.api.NextcloudAPI.convertStreamToTargetEntity(InputStream, Type) concatenates the result of a toString() call
LEST Method com.nextcloud.android.sso.AccountImporter.requestAuthToken(Activity, Intent) throws alternative exception from catch block without history
LEST Method com.nextcloud.android.sso.AccountImporter.requestAuthToken(Fragment, Intent) throws alternative exception from catch block without history
LEST Method com.nextcloud.android.sso.api.NextcloudAPI.getVoidInstance() throws alternative exception from catch block without history
MDM Method com.nextcloud.android.sso.model.SingleSignOnAccount.toString(SingleSignOnAccount) encodes String bytes without specifying the character encoding
NP Null passed for non-null parameter of new java.io.File(String, String) in new com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod(String, Method)
OC Method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.invoke(NextcloudAPI, Object[]) manually casts the right hand side of an assignment more specifically than needed
RCN Nullcheck of inputStream at line 173 of value previously dereferenced in com.nextcloud.android.sso.api.NextcloudAPI.convertStreamToTargetEntity(InputStream, Type)
RFI Method com.nextcloud.android.sso.api.NextcloudAPI.getVoidInstance() uses AccessibleObject.setAccessible to modify accessibility of classes
SPP Method com.nextcloud.android.sso.aidl.IInputStreamService$Stub.asInterface(IBinder) checks a reference for null before calling instanceof

Internationalization Warnings

Code Warning
Dm Found reliance on default encoding in com.nextcloud.android.sso.api.NextcloudAPI.convertStreamToTargetEntity(InputStream, Type): new java.io.InputStreamReader(InputStream)
Dm Found reliance on default encoding in com.nextcloud.android.sso.model.SingleSignOnAccount.toString(SingleSignOnAccount): new String(byte[])

Multithreaded correctness Warnings

Code Warning
JLM Synchronization performed on java.util.concurrent.atomic.AtomicBoolean in com.nextcloud.android.sso.api.AidlNetworkRequest.waitForApi()
JLM Synchronization performed on java.util.concurrent.atomic.AtomicBoolean in com.nextcloud.android.sso.api.AidlNetworkRequest$1.onServiceConnected(ComponentName, IBinder)
JLM Synchronization performed on java.util.concurrent.ConcurrentHashMap in retrofit2.NextcloudRetrofitApiBuilder.loadServiceMethod(Method)

Performance Warnings

Code Warning
PSC Method com.nextcloud.android.sso.aidl.NextcloudRequest$Builder.setParameter(Collection) does not presize the allocation of a collection
PSC Method com.nextcloud.android.sso.aidl.NextcloudRequest$Builder.setParameter(Map) does not presize the allocation of a collection
UCPM Method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parsePathParameters() passes constant String of length 1 to character overridden method
UCPM Method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parsePathParameters() passes constant String of length 1 to character overridden method
WMI com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.invoke(NextcloudAPI, Object[]) makes inefficient use of keySet iterator instead of entrySet iterator

Security Warnings

Code Warning
SECOBDES Object deserialization is used in com.nextcloud.android.sso.api.AidlNetworkRequest.deserializeObject(InputStream)
SECOBDES Object deserialization is used in com.nextcloud.android.sso.api.AidlNetworkRequest.deserializeObjectV2(InputStream)
SECOBDES Object deserialization is used in com.nextcloud.android.sso.api.AidlNetworkRequest.deserializeObjectV2(InputStream)
SECOBDES Object deserialization is used in com.nextcloud.android.sso.model.SingleSignOnAccount.fromString(String)
SECPR This random generator (java.lang.Math.random()) is predictable

Dodgy code Warnings

Code Warning
EXS Unconstrained method com.nextcloud.android.sso.api.NextcloudAPI.convertStreamToTargetEntity(InputStream, Type) converts checked exception to unchecked
EXS Unconstrained method com.nextcloud.android.sso.api.NextcloudAPI.getVoidInstance() converts checked exception to unchecked
EXS Unconstrained method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.bodyToStream(RequestBody) converts checked exception to unchecked
STT This method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parsePathParameters() parses a String that is a field
STT This method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parsePathParameters() parses a String that is a field
STT This method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parsePathParameters() parses a String that is a field
UP Static or private method com.nextcloud.android.sso.AccountImporter.onRequestPermissionsResult(int, String[], int[], Activity, Fragment) has unused parameters
UP Static or private method com.nextcloud.android.sso.api.NextcloudRetrofitServiceMethod.parseHttpMethodAndPath(String, String, boolean) has unused parameters
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse.capabilities
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse.version
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities.theming
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.background
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.color
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.favicon
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.logo
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.name
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.slogan
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsCapabilities$OcsTheming.url
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.edition
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.extendedSupport
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.macro
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.major
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.minor
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsCapabilitiesResponse$OcsVersion.string
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsResponse.ocs
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsResponse$OcsWrapper.data
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsResponse$OcsWrapper.meta
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsResponse$OcsWrapper$OcsMeta.message
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsResponse$OcsWrapper$OcsMeta.status
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.address
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.email
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.enabled
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.groups
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.language
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.lastLogin
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.locale
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.phone
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.quota
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.twitter
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser.website
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser$OcsQuota.free
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser$OcsQuota.total
UuF Unused public or protected field: com.nextcloud.android.sso.model.ocs.OcsUser$OcsQuota.used

Details

CLI_CONSTANT_LIST_INDEX: Method accesses list or array with constant 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.

DM_DEFAULT_ENCODING: Reliance on 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 behaviour to vary between platforms. Use an alternative API and specify a charset name or Charset object explicitly.

EXS_EXCEPTION_SOFTENING_NO_CONSTRAINTS: Unconstrained method converts checked exception to unchecked

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.

FCBL_FIELD_COULD_BE_LOCAL: Class defines fields that are used only as locals

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.

HE_EQUALS_USE_HASHCODE: Class defines equals() and uses Object.hashCode()

This class overrides equals(Object), but does not override hashCode(), and inherits the implementation of hashCode() from java.lang.Object (which returns the identity hash code, an arbitrary value assigned to the object by the VM).  Therefore, the class is very likely to violate the invariant that equal objects must have equal hashcodes.

If you don't think instances of this class will ever be inserted into a HashMap/HashTable, the recommended hashCode implementation to use is:

public int hashCode() {
    assert false : "hashCode not designed";
    return 42; // any arbitrary constant will do
}

IMC_IMMATURE_CLASS_BAD_SERIALVERSIONUID: Class defines a computed serialVersionUID that doesn't equate to the calculated value

This serializable class defines a serialVersionUID that appears to be a computed value, however the value does not match the computed value, and thus losses it's value as version indicator. Either create a custom value like 1, 2, 3, 4.. etc, or recompute the serialVersionUID using your IDE.

ISB_TOSTRING_APPENDING: Method concatenates the result of a toString() call

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.

JLM_JSR166_UTILCONCURRENT_MONITORENTER: Synchronization performed on util.concurrent instance

This method performs synchronization 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.

LEST_LOST_EXCEPTION_STACK_TRACE: Method throws alternative exception from catch block without history

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);
}

MDM_STRING_BYTES_ENCODING: Method encodes String bytes without specifying the character 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.

NM_METHOD_NAMING_CONVENTION: Method names should start with a lower case letter

Methods should be verbs, in mixed case with the first letter lowercase, with the first letter of each internal word capitalized.

NP_NULL_PARAM_DEREF: Method call passes null for non-null parameter

This method call passes a null value for a non-null method parameter. Either the parameter is annotated as a parameter that should always be non-null, or analysis has shown that it will always be dereferenced.

OC_OVERZEALOUS_CASTING: Method manually casts the right hand side of an assignment more specifically than needed

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.

PSC_PRESIZE_COLLECTIONS: Method does not presize the allocation of a collection

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

RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE: Nullcheck of value previously dereferenced

A value is checked here to see whether it is null, but this value can't be null because it was previously dereferenced and if it were null a null pointer exception would have occurred at the earlier dereference. Essentially, this code and the previous dereference disagree as to whether this value is allowed to be null. Either the check is redundant or the previous dereference is erroneous.

RFI_SET_ACCESSIBLE: Method uses AccessibleObject.setAccessible to modify accessibility of classes

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 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.

OBJECT_DESERIALIZATION: Object deserialization is used in {1}

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)

PREDICTABLE_RANDOM: Predictable pseudorandom number generator

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:

A quick fix could be to replace the use of java.util.Random with something stronger, such as java.security.SecureRandom.

Vulnerable Code:

String generateSecretToken() {
    Random r = new Random();
    return Long.toHexString(r.nextLong());
}

Solution: 

import org.apache.commons.codec.binary.Hex;

String generateSecretToken() {
    SecureRandom secRandom = new SecureRandom();

    byte[] result = new byte[32];
    secRandom.nextBytes(result);
    return Hex.encodeHexString(result);
}


References
Cracking Random Number Generators - Part 1 (https://jazzy.id.au)
CERT: MSC02-J. Generate strong random numbers
CWE-330: Use of Insufficiently Random Values
Predicting Struts CSRF Token (Example of real-life vulnerability and exploitation)

SPP_NULL_BEFORE_INSTANCEOF: Method checks a reference for null before calling 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.

STT_STRING_PARSING_A_FIELD: This method parses a String that is 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_USE_CHARACTER_PARAMETERIZED_METHOD: Method passes constant String of length 1 to character overridden 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');
}

UP_UNUSED_PARAMETER: Static or private method has unused parameters

This method defines parameters that are never used. As this method is either static or private, and can't be derived from, it is safe to remove these parameters and simplify your method. You should consider, while unlikely, that this method may be used reflectively, and thus you will want to change that call as well. In this case, it is likely that once you remove the parameter, there will be a chain of method calls that have spent time creating this parameter and passing it down the line. All of this may be able to be removed.

UUF_UNUSED_PUBLIC_OR_PROTECTED_FIELD: Unused public or protected field

This field is never used.  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.

WMI_WRONG_MAP_ITERATOR: Inefficient use of keySet iterator instead of entrySet iterator

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.