Poor code quality leads to unpredictable behavior. From a user's perspective that often manifests itself as poor usability. For an attacker it provides an opportunity to stress the system in unexpected ways.
Camera
object after it has already been released.Camera
object after the it has already been released. Any further references to the Camera
object without reacquiring the resource will throw an exception, and can cause the application to crash if the exception is not caught.startPreview()
is called on the previously-released Camera
object.
public class ReuseCameraActivity extends Activity {
private Camera cam;
...
private class CameraButtonListener implements OnClickListener {
public void onClick(View v) {
if (toggle) {
cam.stopPreview();
cam.release();
}
else {
cam.startPreview();
}
toggle = !toggle;
}
}
...
}
start()
is called on the previously-released media resource.
public class ReuseMediaPlayerActivity extends Activity {
private MediaPlayer mp;
...
private class PauseButtonListener implements OnClickListener {
public void onClick(View v) {
if (paused) {
mp.pause();
mp.release();
}
else {
mp.start();
}
paused = !paused;
}
}
...
}
flushUpdates()
to commit the changes to disk. The method properly closes the database handler after writing updates to the database. However, when flushUpdates()
is called again, the database object is referenced again before reinitializing it.
public class ReuseDBActivity extends Activity {
private myDBHelper dbHelper;
private SQLiteDatabase db;
@Override
public void onCreate(Bundle state) {
...
db = dbHelper.getWritableDatabase();
...
}
...
private void flushUpdates() {
db.insert(cached_data); // flush cached data
dbHelper.close();
}
...
}
...
<script src="http://applicationserver.application.com/lib/jquery/jquery-1.4.2.js" type="text/javascript"></script>
...
String
may lead to data loss.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.
...
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
...
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.notify()
is called.notify()
.notifyJob()
calls notify()
.
public synchronized notifyJob() {
flag = true;
notify();
}
...
public synchronized waitForSomething() {
while(!flag) {
try {
wait();
}
catch (InterruptedException e)
{
...
}
}
...
}
wait()
, but it is possible that notify()
will notify a different thread than the intended one.run()
method instead of calling start()
.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.run()
instead of start()
.
Thread thr = new Thread() {
public void run() {
...
}
};
thr.run();
stop()
method, potentially leaking resources.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.Thread.stop()
.
...
public static void main(String[] args){
...
Thread thr = new Thread() {
public void run() {
...
}
};
...
thr.start();
...
thr.stop();
...
}
clone()
method but does not implement the Cloneable
interface.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.clone()
for this class will result in a CloneNotSupportedException.
public class Kibitzer {
public Object clone() throws CloneNotSupportedException {
...
}
}
ICloneable
interface specifies a weak contract for its Clone
method and should be avoided.ICloneable
interface does not guarantee deep cloning, classes that implement it may not behave as expected when they are cloned. Classes that implement ICloneable
and perform only shallow-cloning (copies only the object, which includes existing references to other objects) may result in unexpected behavior. Because deep-cloning (copies the object and all referenced objects) is typically the assumed behavior of a clone method, the use of the ICloneable
interface is error prone and should be avoided.clone()
method within the class calls a function that can be overridden.clone()
function calls an overridable function, it may cause the clone to be left in a partially initialized state, or become corrupted.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(){
...
}
}
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.equals()
method can result in unexpected behavior.equals()
method should be called instead of the operators ==
and !=
. The Java Specification states about boxing conversions: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.
...
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
...
}
...
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.NaN
is always an error.NaN
it is always evaluated as false
, except for the !=
operator, which always evaluates to true
since NaN
is unordered.NaN
.
...
if (result == Double.NaN){
//something went wrong
throw new RuntimeException("Something went wrong, NaN found");
}
...
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.this
reference prior to the object being fully initialized, which can in turn lead to a vulnerability.
...
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;
}
}
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");
}
}
}
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.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.GetType().FullName == "CompanyX.Transaction.Monetary")
{
processTransaction(inputReader);
}
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();
...
}
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::class.qualifiedName == "com.example.TrustedClass") {
input = inputReader.getInput()
...
}
x = NULL
and x != NULL
will always be false.NULL
is indeterminate. It is not equal to anything, not even another NULL
value. Also, a null
value is never not equal to another value.Example 2: The following statement will always be false.
checkNull BOOLEAN := x = NULL;
checkNotNull BOOLEAN := x != NULL;
equals()
method, not ==
or !=
.==
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.
if (args[0] == STRING_CONSTANT) {
logger.info("miracle");
}
==
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.