Failure to follow guideline/specification

From OWASP
Revision as of 21:02, 13 February 2009 by KirstenS (Talk | contribs)

Jump to: navigation, search

This is a Vulnerability. To view all vulnerabilities, please see the Vulnerability Category page.


This article includes content generously donated to OWASP by Fortify.JPG.

ASDR Table of Contents

Last revision (mm/dd/yy): 02/13/2009


Description

Code Correctness: Call to System.gc()

Explicit requests for garbage collection are a bellwether indicating likely performance problems.

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.

Code Correctness: Call to Thread.run()

The program calls a thread's run() method instead of calling start().

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.

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

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

Code Correctness: Class Does Not Implement Cloneable

This class implements a clone() method but does not implement Cloneable.

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.


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

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

Code Correctness: Double-Checked Locking

Double-checked locking is an incorrect idiom that does not achieve the intended effect. 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.


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

Code Correctness: Erroneous String Compare

Strings should be compared with the equals() method, not == or !=. 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.

The following branch will never be taken.

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

Code Correctness: Erroneous finalize() Method

This finalize() method does not call super.finalize().

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

The statement above is not completely correct. The Java Language Specification 3.0 section 12.6.1 states: "This should always be done, unless it is the programmer's intent to nullify the actions of the finalizer in the superclass."

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

 protected void finalize() {
                discardNative();
 }

Code Correctness: Misspelled Method Name

This looks like an effort to override a common Java method, but it probably does not have the intended effect. This method's name is similar to a common Java method name, but it is either spelled incorrectly or the argument list causes it to not override the intended method.

The following method is meant to override Object.equals():

	public boolean equals(Object obj1, Object obj2) {
	  ...
	}

But since Object.equals() only takes a single argument, the method above is never called.

Code Correctness: null Argument to equals()

The expression obj.equals(null) should always be false.

The program uses the equals() method to compare an object with null. The contract of the equals() method requires this comparison to always return false[3].

Dead Code: Broken Override

This method fails to override a similar method in its superclass because their parameter lists do not match. This method declaration looks like an attempt to override a method in a superclass, but the parameter lists do not match, so the superclass method is not overridden.

The class DeepFoundation is meant to override the method getArea() in its parent class, but the parameter lists are out of sync.

	public class Foundation
	{
	  public int getArea() {
		...
	  }
	}
	
	class DeepFoundation extends Foundation
	{
	  public int getArea(int a) {
		...
	  }
	}

Dead Code: Expression is Always False

This expression will always evaluate to false. This expression will always evaluate to false; the program could be rewritten in a simpler form. The nearby code may be present for debugging purposes, or it may not have been maintained along with the rest of the program. The expression may also be indicative of a bug earlier in the method.

The following method never sets the variable secondCall after initializing it to false. (The variable firstCall is mistakenly used twice.) The result is that the expression firstCall && secondCall will always evaluate to false, so setUpDualCall() will never be invoked.

	public void setUpCalls() {
	  boolean firstCall = false;
	  boolean secondCall = false;
	
	  if (fCall > 0) {
		setUpFCall();
		firstCall = true;
	  }
	  if (sCall > 0) {
		setUpSCall();
		firstCall = true;
	  }
	
	  if (firstCall && secondCall) {
		setUpDualCall();
	  }
	}


Risk Factors

TBD

Examples

TBD


Related Attacks


Related Vulnerabilities


Related Controls


Related Technical Impacts


References