Even if castor creates a lock on an object it does not prevent other threads from reading or writing to it. Read-only queries are also about 7 times faster compared with default shared mode.

Example 1: The following example specifies the query mode as SHARED which allows both read and write access.

results = query.execute(Database.SHARED);

By default Castor executes queries in shared mode. Since shared mode allows both read and write access, it is unclear what kind of operation the query is intended for. If the object is going to be used in a read-only context, shared access adds unnecessary performance overhead.

Example 1: The following example does not specify a query mode.

results = query.execute();    //missing query mode

ClassLoader manipulation allows an attacker to access and modify the underlying application server settings. On certain applications servers like Tomcat 8, an attacker may tweak these settings to upload a web shell and execute arbitrary commands.

When data from a byte array is converted into a String, it is unspecified what will happen to any data that is outside of the applicable character set. This can lead to data being lost, or a decrease in the level of security when binary data is needed to ensure proper security measures are followed.

Example 1: The following code converts data into a String in order to create a hash.

  ...
  FileInputStream fis = new FileInputStream(myFile);
  byte[] byteArr = byte[BUFSIZE];
  ...
  int count = fis.read(byteArr);
  ...
  String fileString = new String(byteArr);
  String fileSHA256Hex = DigestUtils.sha256Hex(fileString);
  // use fileSHA256Hex to validate file
  ...

Assuming the size of the file is less than BUFSIZE, this works fine as long as the information in myFile is encoded the same as the default character set, however if it's using a different encoding, or is a binary file, it will lose information. This in turn will cause the resulting SHA hash to be less reliable, and could mean it's far easier to cause collisions, especially if any data outside of the default character set is represented by the same value, such as a question mark.

There is no way to specify which thread will be awakened by calls to notify().

Example 1: In the following code, notifyJob() calls notify().

public synchronized notifyJob() {
flag = true;
notify();
}
...
public synchronized waitForSomething() {
while(!flag) {
    try {
        wait();
    }
    catch (InterruptedException e)
    {
        ...
    }
}
...
}

In this case, the developer intends to wake up the thread that calls wait(), but it is possible that notify() will notify a different thread than the intended one.

If multiple threads are trying to obtain a lock on a resource, calling sleep() while holding a lock can cause all of the other threads to wait for the resource to be released, which can result in degraded performance and deadlock.

Example 1: The following code calls sleep() while holding a lock.

ReentrantLock rl = new ReentrantLock();
...
rl.lock();
Thread.sleep(500);
...
rl.unlock();

At some point in every Java developer's career, a problem surfaces that appears to be so mysterious, impenetrable, and impervious to debugging that there seems to be no alternative but to blame the garbage collector. Especially when the bug is related to time and state, there may be a hint of empirical evidence to support this theory: inserting a call to System.gc() sometimes seems to make the problem go away.

In almost every case we have seen, calling System.gc() is the wrong thing to do. In fact, calling System.gc() can cause performance problems if it is invoked too often.

In most cases a direct call to a Thread object's run() method is a bug. The programmer intended to begin a new thread of control, but accidentally called run() instead of start(), so the run() method will execute in the caller's thread of control.

Example 1: The following excerpt from a Java program mistakenly calls run() instead of start().

    Thread thr = new Thread() {
      public void run() {
        ...
      }
    };

  thr.run();

In most cases a direct call to a Thread object's stop() method is a bug. The programmer intended to stop a thread from running, but was unaware that this is not a suitable way to stop a thread. The stop() function within Thread causes a ThreadDeath exception anywhere within the Thread object, likely leaving objects in an inconsistent state and potentially leaking resources. Due to this API being inherently unsafe, its use was deprecated long ago.

Example 1: The following excerpt from a Java program mistakenly calls Thread.stop().

  ...
  public static void main(String[] args){
    ...
    Thread thr = new Thread() {
      public void run() {
        ...
      }
    };
    ...
    thr.start();
    ...
    thr.stop();
    ...
  }

It appears that the programmer intended for this class to implement the Cloneable interface because it implements a method named clone(). However, the class does not implement the Cloneable interface and the clone() method will not behave correctly.

Example 1: Calling clone() for this class will result in a CloneNotSupportedException.

public class Kibitzer {
  public Object clone() throws CloneNotSupportedException {
    ...
  }
}

When comparing objects, developers usually want to compare properties of objects. However, calling equals() on a class (or any super class/interface) that does not explicitly implement equals() results in a call to the equals() method inherited from java.lang.Object. Instead of comparing object member fields or other properties, Object.equals() compares two object instances to see if they are the same. Although there are legitimate uses of Object.equals(), it is often an indication of buggy code.

Example 1:

public class AccountGroup
{
	private int gid;

	public int getGid()
	{
		return gid;
	}

	public void setGid(int newGid)
	{
		gid = newGid;
	}
}
...
public class CompareGroup
{
	public boolean compareGroups(AccountGroup group1, AccountGroup group2)
	{
		return group1.equals(group2);   //equals() is not implemented in AccountGroup
	}
}

When a clone() function calls an overridable function, it may cause the clone to be left in a partially initialized state, or become corrupted.

Example 1: The following clone() function calls a method that can be overridden.

  ...
  class User implements Cloneable {
    private String username;
    private boolean valid;
    public Object clone() throws CloneNotSupportedException {
      final User clone = (User) super.clone();
      clone.doSomething();
      return clone;
    }
    public void doSomething(){
      ...
    }
  }

Since the function doSomething() and its enclosing class are not final, it means that the function can be overridden, which may leave the cloned object clone in a partially initialized state, which may lead to errors, if not working around logic in an unexpected way.

When dealing with boxed primitives, when comparing equality, the boxed primitive's equals() method should be called instead of the operators == and !=. The Java Specification states about boxing conversions:

"If the value p being boxed is an integer literal of type int between -128 and 127 inclusive, or the boolean literal true or false, or a character literal between '\u0000' and '\u007f' inclusive, then let a and b be the results of any two boxing conversions of p. It is always the case that a == b."

This means that if a boxed primitive is used (other than Boolean or Byte), only a range of values will be cached, or memoized. For a subset of values, using == or != will return the correct value, for all other values outside of this subset, this will return the result of comparing the object addresses.

Example 1: The following example uses equality operators on boxed primitives.

  ...
  Integer mask0 = 100;
  Integer mask1 = 100;
  ...
  if (file0.readWriteAllPerms){
    mask0 = 777;
  }
  if (file1.readWriteAllPerms){
    mask1 = 777;
  }
  ...
  if (mask0 == mask1){
    //assume file0 and file1 have same permissions
    ...
  }
  ...

The code in Example 1 uses Integer boxed primitives to try to compare two int values. If mask0 and mask1 are both equal to 100, then mask0 == mask1 will return true. However, when mask0 and mask1 are both equal to 777, now mask0 == maske1 will return false as these values are not within the range of cached values for these boxed primitives.

When a comparison is made to NaN it is always evaluated as false, except for the != operator, which always evaluates to true since NaN is unordered.

Example 1: The following tries to make sure a variable is not NaN.

  ...
  if (result == Double.NaN){
    //something went wrong
    throw new RuntimeException("Something went wrong, NaN found");
  }
  ...

This attempts to verify that result is not NaN, however using the operator == with NaN always results in a value of false, so this check will never throw the exception.

When a constructor calls an overridable function, it may allow an attacker to access the this reference prior to the object being fully initialized, which can in turn lead to a vulnerability.

Example 1: The following calls a method that can be overridden.

  ...
  class User {
    private String username;
    private boolean valid;
    public User(String username, String password){
      this.username = username;
      this.valid = validateUser(username, password);
    }
    public boolean validateUser(String username, String password){
      //validate user is real and can authenticate
      ...
    }
    public final boolean isValid(){
      return valid;
    }
  }

Since the function validateUser and the class are not final, it means that they can be overridden, and then initializing a variable to the subclass that overrides this function would allow bypassing of the validateUser functionality. For example:

  ...
  class Attacker extends User{
    public Attacker(String username, String password){
      super(username, password);
    }
    public boolean validateUser(String username, String password){
      return true;
    }
  }
  ...
  class MainClass{
    public static void main(String[] args){
      User hacker = new Attacker("Evil", "Hacker");
      if (hacker.isValid()){
        System.out.println("Attack successful!");
      }else{
        System.out.println("Attack failed");
      }
    }
  }

The code in Example 1 prints "Attack successful!", since the Attacker class overrides the validateUser() function that is called from the constructor of the superclass User, and Java will first look in the subclass for functions called from the constructor.

Many talented individuals have spent a great deal of time pondering ways to make double-checked locking work in order to improve performance. None have succeeded.

Example 1: At first blush it may seem that the following bit of code achieves thread safety while avoiding unnecessary synchronization.

if (fitz == null) {
  synchronized (this) {
    if (fitz == null) {
      fitz = new Fitzer();
    }
  }
}
return fitz;

The programmer wants to guarantee that only one Fitzer() object is ever allocated, but does not want to pay the cost of synchronization every time this code is called. This idiom is known as double-checked locking.

Unfortunately, it does not work, and multiple Fitzer() objects can be allocated. See The "Double-Checked Locking is Broken" Declaration for more details [1].

Attackers can deliberately duplicate class names in order to cause a program to execute malicious code. For this reason, class names are not good type identifiers and should not be used as the basis for granting trust to a given object.

Example 1: The following code determines whether to trust input from an inputReader object based on its class name. If an attacker can supply an implementation of inputReader that executes malicious commands, this code cannot differentiate the benign and malicious versions of the object.

if (inputReader.getClass().getName().equals("com.example.TrustedClass")) {
   input = inputReader.getInput();
   ...
}

The Java Language Specification states that it is a good practice for a finalize() method to call super.finalize() [1].

Example 1: The following method omits the call to super.finalize().

protected void finalize() {
  discardNative();
}

This field has been annotated with FortifyNonNegative, which is used to indicate that negative values are not permitted.

This program uses == or != to compare two strings for equality, which compares two objects for equality, not their values. Chances are good that the two references will never be equal.

Example 1: The following branch will never be taken.

  if (args[0] == STRING_CONSTANT) {
      logger.info("miracle");
  }

The == and != operators will only behave as expected when they are used to compare strings contained in objects that are equal. The most common way for this to occur is for the strings to be interned, whereby the strings are added to a pool of objects maintained by the String class. Once a string is interned, all uses of that string will use the same object and equality operators will behave as expected. All string literals and string-valued constants are interned automatically. Other strings can be interned manually be calling String.intern(), which will return a canonical instance of the current string, creating one if necessary.