Audit - Rules - Threads and SynchronizationDescriptionThis group contains audit rules that look for possible problems having to do with the use of threads and synchronization. |
Rules: |
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);
}
}
}
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);
}
}
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();
}
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();
}
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() {
...
}
}
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();
}
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();
}
}
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() ){
}
}
Summary
Do not use Calendar
objects without synchronization.
Description
This rule looks for a places where Calendar
objects are used without synchronization.
Security ImplicationsCalendars
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;
}
Summary
Do not use DateFormat
objects without synchronization.
Description
This rule looks for places where a DateFormat
object is used without synchronization.
Security ImplicationsDateFormat
s 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;
}
Summary
If class is subtype of java.util.concurrent.locks.Condition
you should use await method instead of wait method.
Description
This audit rule looks for places where the wait()
method is invoked on a java.util.concurrent.locks.Condition
object.
Security Implications
Invoking the wait()
method on a java.util.concurrent.locks.Condition
object is usually a mistake. This class is specially used for synchronization and provides the method await(), similar to wait() in its name. Accidentially calling wait() instead is a plain error which will result in the application behaving in an unexpected way and possibly becoming vulnurable to an attack. Asynchronous environment is especially vulnurable to such errors as deadlock may occur and ultimately lead to denial-of-service state.
Example
The following code will be marked as a violation because the wait()
method used:
public void myFunction( Condition cond )
{
try{
cond.wait();
catch( InterruptException e){
return;
}
}