Failure to follow guideline/specification

From OWASP
Revision as of 08:45, 17 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/17/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.

Example

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.

Example

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.

Example

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.

Example

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

Example

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.

Example

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.

Example

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.

Example

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();
	  }
	}

Dead Code: Expression is Always True

This expression will always evaluate to true; 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.

Example

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

	public void setUpCalls() {
	  boolean firstCall = true;
	  boolean secondCall = true;
	
	  if (fCall < 0) {
		cancelFCall();
		firstCall = false;
	  }
	  if (sCall < 0) {
		cancelSCall();
		firstCall = false;
	  }
	
	  if (firstCall || secondCall) {
		setUpForCall();
	  }
	}

Dead Code: Unused Field

This field is never accessed, except perhaps by dead code. It is likely that the field is simply vestigial, but it is also possible that the unused field points out a bug.

Example 1

The field named glue is not used in the following class. The author of the class has accidentally put quotes around the field name, transforming it into a string constant.

	public class Dead {
	
	  String glue;
	
	  public String getGlue() {
		return "glue";
	  }
	
	}

Example 2

The field named glue is used in the following class, but only from a method that is never called.

	public class Dead {
	
	  String glue;
	
	  private String getGlue() {
		return glue;
	  }
	
	}

Dead Code: Unused Method

This method is never called or is only called from other dead code.

Example 1

In the following class, the method doWork() can never be called.

	public class Dead {
	  private void doWork() {
		System.out.println("doing work");
	  }
	  public static void main(String[] args) {
		System.out.println("running Dead");
	  }
	}

Example 2

In the following class, two private methods call each other, but since neither one is ever invoked from anywhere else, they are both dead code.

	public class DoubleDead {
	  private void doTweedledee() {
		doTweedledumb();
	  }
	  private void doTweedledumb() {
		doTweedledee();
	  }
	  public static void main(String[] args) {
		System.out.println("running DoubleDead");
	  }
	}

(In this case it is a good thing that the methods are dead: invoking either one would cause an infinite loop.)

EJB Bad Practices: Use of AWT/Swing

The program violates the Enterprise JavaBeans specification by using AWT/Swing.

The Enterprise JavaBeans specification requires that every bean provider follow a set of programming guidelines designed to ensure that the bean will be portable and behave consistently in any EJB container [4].

In this case, the program violates the following EJB guideline:

"An enterprise bean must not use the AWT functionality to attempt to output information to a display, or to input information from a keyboard."

A requirement that the specification justifies in the following way:

"Most servers do not allow direct interaction between an application program and a keyboard/display attached to the server system."

EJB Bad Practices: Use of Class Loader

The program violates the Enterprise JavaBeans specification by using the class loader.

The Enterprise JavaBeans specification requires that every bean provider follow a set of programming guidelines designed to ensure that the bean will be portable and behave consistently in any EJB container. [4]

In this case, the program violates the following EJB guideline:

"The enterprise bean must not attempt to create a class loader; obtain the current class loader; set the context class loader; set security manager; create a new security manager; stop the JVM; or change the input, output, and error streams."

A requirement that the specification justifies in the following way:

"These functions are reserved for the EJB container. Allowing the enterprise bean to use these functions could compromise security and decrease the container's ability to properly manage the runtime environment."

EJB Bad Practices: Use of Sockets

The program violates the Enterprise JavaBeans specification by listening on a socket or accept connections on a socket. However it can act as a network socket client.

The Enterprise JavaBeans specification requires that every bean provider follow a set of programming guidelines designed to ensure that the bean will be portable and behave consistently in any EJB container [4].

In this case, the program violates the following EJB guideline:

"An enterprise bean must not attempt to listen on a socket, accept connections on a socket, or use a socket for multicast."

A requirement that the specification justifies in the following way:

"The EJB architecture allows an enterprise bean instance to be a network socket client, but it does not allow it to be a network server. Allowing the instance to become a network server would conflict with the basic function of the enterprise bean – to serve the EJB clients."

Risk Factors

TBD

Examples

TBD


Related Attacks


Related Vulnerabilities


Related Controls


Related Technical Impacts


References