Java Developer Tools

Audit - Rules - Threads and Synchronization

Description
This group contains audit rules that look for possible problems having to do with the use of threads and synchronization.

Rules:

Details

Concurrent Modification

Summary
Removing from the collection being iterated over and continuing iteration afterwards results in an exception.

Description
This audit rule violates the cases when elements are removed from a collection while still iterating over it. Precisely, calling Iterator#next() after remove() was called results in an exception. Correct way of dealing with a necessity to do so is to iterate over the duplicate of a collection while removing found objects from the original or to use the iterators remove() method if it is known to be supported.

Security Implications
For fail-fast collections concurrent modifications result in ConcurrentModificationException, which could create an unexpected situation in the code execution and thus be a potential security threat.

Example
The following method removes elements from a collection while iterating over it and thus would be marked as violation:

    public void removeByPattern(Collection c, String pattern) {
        for (Iterator i = c.iterator(); i.hasNext();) {
            String val = (String) i.next();
            if (val.contains(pattern)) {
                c.remove(val);
            }
        }
    }

Field Access Protection

Summary
When a field that is usually accessed in a synchronized context is accessed without synchronization, this indicates an error.

Description
This audit rule looks for unsynchronized accesses to fields that are usually accessed in a synchronized way. A field is considered to match the criteria if synchronization is used to access it in at least 66% of all of the places in which it is accessed.

Security Implications
Partial or incomplete synchronization is the primary source of all deadlocks and race conditions. Multi-threaded parts of data should always be accessed in a synchronized block.

Example
The following class appears to provide synchronized access to the internal list, but remove() method is not synchronized and thus will be marked as violation:

    public class SynchronizedList {
        private final List data = new ArrayList();
        public synchronized void add(Object item) {
            data.add(item);
        }
        public synchronized int size() {
            return data.size();
        }
        public synchronized Object pop() {
            return data.remove(0);
        }
        public void remove(Object item) {
            data.remove(item);
        }
    }

Mismatched Notify

Summary
Do not use notify() or notifyAll() methods without holding locks.

Description
This audit rule looks for invocations of notify() or notifyAll() methods without holding a lock on the object.

Security Implications
Invoking notify() or notifyAll() without holding a lock can lead to throwing an IllegalMonitorStateException.

Example
The following invocation of the notify() method will be marked as a violation because the container method is not synchronized:

    public void method() {
            notify();
    }

Mismatched Wait

Summary
Do not use wait() method without holding locks.

Description
This audit rule looks for invocations of the wait() method without holding a lock on the object.

Security Implications
Invoking wait() without holding a lock can lead to throwing an IllegalMonitorStateException.

Example
The following invocation of the wait method will be marked as a violation because the container method is not synchronized:

    public void method(){
            wait();
    }

Nested Synchronized Calls

Summary
Invoking one synchronized method of an object from another synchronized method of the same object affects the performance of an application.

Description
This audit rule looks for invocations of a synchronized method from another synchronized method in the same class.

Security Implications
Such calls both affect the performance of an application and indicate a poorly designed synchronization aspect of the code, which usually results in synchronization errors that could be exploited to create unexpected states of an application.

Example
The following code would be flagged as a violation because it invokes one synchronized method of an object from another synchronized method of the same object:

    public class SyncDataSource {
        public synchronized Object getData() {
            return internalGetData();
        }
        private synchronized Object internalGetData() {
            ...
        }
    }

Non-atomic File Operations

Summary
Checking a file for existence and then writing into it is a non-atomic operation, the assumption about it being atomic may fail, leading to unexpected consequences.

Description
This audit rule looks for conditional decisions made on the basis of an invocation of java.io.File.exists().

Security Implications
Calling exists() and then performing some file operation based on the result of this call is a non-atomic operation, i.e. the file state of existence can change in the gap of time between exists() call and the subsequent actions. This may result in an unexpected behavior of an application that could possibly compromise its security.

Example
The following code would be flagged as a violation because it makes a file writing decision based on the result of exists() call:

    File lock = new File(".lock");
    if (!lock.exists()) {
        lock.createNewFile();
        ...
        lock.delete();
    }

Start Method Invoked In Constructor

Summary
Do not invoke java.lang.Thread.start() method inside constructors.

Description
This audit rule looks for invocations of java.lang.Thread.start() method inside constructors.

Security Implications
If the class is extended/subclassed, the method start() will be invoked before the constructor of the subclass has finished, potentially allowing the new thread to see the object in an inconsistent state.

Example
The following invocation of the start method will be marked as a violation because it is called inside the constructor:

    public class SimleClass {
        public SimleClass() {
            new Thread() {
                public void run(){
                }
            }.start();
        }
    }

Synchronization On getClass Method

Summary
You should avoid synchronization on getClass() method.

Description
This rule looks for places where the result of the getClass() method is used for synchronization.

Security Implications
If this class is subclassed, subclasses will synchronize on a different class object, which isn't likely what was intended.

Example
The following code is synchronized on getClass method and thuswill be marked as a violation:

    public void methodOne(){
        synchronized ( getClass() ){
        }
    }

Usage Of Static Calendar

Summary
Do not use Calendar objects without synchronization.

Description
This rule looks for a places where Calendar objects are used without synchronization.

Security Implications
Calendars are inherently unsafe for multithread usage. If this object is used without proper synchronizations, ArrayIndexOutOfBoundsException can be thrown.

Example
The following usage of a Calendar object will be marked as a violation because it should be used in synchronized block:

    private static Calendar data;
    ...
    public Calendar getCalendar() {
        return data;
    }

Usage Of Static Date Format

Summary
Do not use DateFormat objects without synchronization.

Description
This rule looks for places where a DateFormat object is used without synchronization.

Security Implications
DateFormats are inherently unsafe for multithread usage. Sharing a single instance across thread boundaries without proper synchronization can result in erratic behavior of the application.

Example
The following usage of a DateFormat object will be marked as a violation because it should be used in a synchronized block:

    private static DateFormat data;
    ...
    public DateFormat getFormat() {
        return data;
    }

wait() Invoked Instead of await()

Authentication required

You need to be signed in with Google+ to do that.

Signing you in...

Google Developers needs your permission to do that.