The assert Solidity function is intended to only check on statements that evaluate to true. Having a false statement passed into this function points at the code not functioning correctly or the function being misused, for instance, to validate input.

Example 1: The following code uses the assert function to check on a false statement.

contract A {
    B b = new B(7);

    function checkWithAssert(){
        assert(b.retValue() == 21);
        ...
    }

}

contract B {
    uint _par;
    constructor(uint par){
        _par = par;
    }

    function retValue() returns(uint){
        return _par;
    }
}

This expression will always be non-null because it references a function pointer rather than the return value of the function.

Example 1: The following conditional will never fire. The predicate getChunk == NULL will always be false because getChunk is the name of a function defined in the program.

  if (getChunk == NULL)
    return ERR;

Because local variables are allocated on the stack, when a program returns a pointer to a local variable, it is returning a stack address. A subsequent function call is likely to re-use this same stack address, thereby overwriting the value of the pointer, which no longer corresponds to the same variable since a function's stack frame is invalidated when it returns. At best this will cause the value of the pointer to change unexpectedly. In many cases it causes the program to crash the next time the pointer is dereferenced. The problem can be hard to debug because the cause of the problem is often far removed from the symptom.

Example 1: The following function returns a stack address.

char* getName() {
  char name[STR_MAX];
  fillInName(name);
  return name;
}

Static methods cannot be overridden by definition, since they belong to the class rather than an instance of the class. Although there are cases where it looks like a static method has been overridden in a subclass, which may cause confusion and can lead to the incorrect version of the method being called.

Example 1: The following tries to define an API for authenticating users.

class AccessLevel{
  public static final int ROOT = 0;
  //...
  public static final int NONE = 9;
}
//...
class User {
  private static int access;
  public User(){
    access = AccessLevel.ROOT;
  }
  public static int getAccessLevel(){
    return access;
  }
  //...
}
class RegularUser extends User {
  private static int access;
  public RegularUser(){
    access = AccessLevel.NONE;
  }
  public static int getAccessLevel(){
    return access;
  }
  public static void escalatePrivilege(){
    access = AccessLevel.ROOT;
  }
  //...
}
//...
class SecureArea {
  //...
  public static void doRestrictedOperation(User user){
    if (user instanceof RegularUser){
      if (user.getAccessLevel() == AccessLevel.ROOT){
        System.out.println("doing a privileged operation");
      }else{
        throw new RuntimeException();
      }
    }
  }
}

At first glance, this code looks fine. However, since we are calling the method getAccessLevel() against the instance user and not against the classes User or RegularUser, it will mean that this condition will always return true, and the restricted operation will be performed even though instanceof was used in order to get into this part of the if/else block.

Code Correctness: Incorrect Serializable Method Signature issues occur when a serializable class creates a serialization or deserialization function but does not follow the correct signatures:

  private void writeObject(java.io.ObjectOutputStream out) throws IOException;
  private void readObject(java.io.ObjectInputStream in) throws IOException, ClassNotFoundException;
  private void readObjectNoData() throws ObjectStreamException;

Deviating from the method signatures that serialization requires may mean that the method is never called during serialization/deserialization, leading to incomplete serialization/deserialization, or could mean that untrusted code could gain access to the objects.
In the case that there are exceptions that are not thrown, it may mean that serialization/deserialization fails and crashes the application or potentially even fails quietly such that objects may be only partially constructed correctly, leading to flaws that can be extremely difficult to debug. The caller should catch these exceptions such that incorrect serialization/deserialization can be handled properly without a crash or partially constructed objects.

The Java Object Serialization Specification enables developers to manually define Serializable fields for a class by specifying them in the serialPersistentFields array. This feature will only work if serialPersistentFields is declared as private, static, and final.

Example 1: The following declaration of serialPersistentFields will not be used to define Serializable fields because it is not private, static, and final.

class List implements Serializable {
public ObjectStreamField[] serialPersistentFields = { new ObjectStreamField("myField", List.class) };
...
}

Calling Object.equals() against an array is a mistake in most cases, since this will check the equality of the arrays' addresses, instead of the equality of the arrays' elements, and should usually be replaced by java.util.Arrays.equals().

Example 1: The following tries to check two arrays using the Object.equals() function.

...
  int[] arr1 = new int[10];
  int[] arr2 = new int[10];
  ...
  if (arr1.equals(arr2)){
    //treat arrays as if identical elements
  }
...

This will almost always result in code that is never executed, unless at some point there is an assignment of one array to the other.

Certain families of functions are implemented as functions on some platforms and macros on others. If the functions rely on a shared resource that is maintained internally rather than passed in when they are invoked, they must be used in the same program scope because the otherwise the shared resource will be inaccessible.

Example 1: The following code uses pthread_cleanup_push() to push the function routine onto the calling thread's cleanup stack and returns. Since pthread_cleanup_push() and its partner function pthread_cleanup_pop() are implemented as macros on platforms other than IBM AIX, the data structure created by pthread_cleanup_push() will not be accessible to subsequent calls to pthread_cleanup_pop(). The code will either fail to compile or behave incorrectly at runtime on all platforms where these functions are implemented as macros.

void helper() {
   ...
   pthread_cleanup_push (routine, arg);
}

Do not explicitly deallocate stack memory. A function that defines a stack buffer will automatically deallocate the buffer when the function returns.
Example 1:

void clean_up()
{
    char tmp[256];
    ...
    free(tmp);
    return;
}

Explicitly freeing stack memory can corrupt memory allocation data structures. It can lead to abnormal program termination or further data corruption.

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 1: 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 in Example 1 is never called.

The .NET runtime will permit the serialization of any object that declares the [Serializable] attribute. If the class can be serialized using the default serialization methods defined by the .NET framework, this is both necessary and sufficient for the object to be correctly serialized. If the class requires custom serialization methods, it must also implement the ISerializable interface. However, the class must still declare the [Serializable] attribute.

Example 1: The CustomStorage class implements the ISerializable interface. However, because it fails to declare the [Serializable] attribute, it will not be serialized.

public class CustomStorage: ISerializable {
	...
}

Forwarding an HttpServletRequest, redirecting an HttpServletResponse, or flushing the servlet's output stream buffer causes the associated stream to commit. Any subsequent buffer resets or stream commits, such as additional flushes or redirects, will result in IllegalStateExceptions.

Furthermore, Java servlets allow data to be written to the response stream using either ServletOutputStream or PrintWriter, but not both. Calling getWriter() after having called getOutputStream(), or vice versa, will also cause an IllegalStateException.



At runtime, an IllegalStateException prevents the response handler from running to completion, effectively dropping the response. This can cause server instability, which is a sign of an improperly implemented servlet.

Example 1: The following code redirects the servlet response after its output stream buffer has been flushed.

public class RedirectServlet extends HttpServlet {
    public void doGet(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException {
        ...
        OutputStream out = res.getOutputStream();
        ...
        // flushes, and thereby commits, the output stream
        out.flush();
        out.close();        // redirecting the response causes an IllegalStateException
        res.sendRedirect("http://www.acme.com");
    }
}

Example 2: Conversely, the following code attempts to write to and flush the PrintWriter's buffer after the request has been forwarded.

public class FlushServlet extends HttpServlet {
    public void doGet(HttpServletRequest req, HttpServletResponse res) throws ServletException, IOException {
        ...
        // forwards the request, implicitly committing the stream
        getServletConfig().getServletContext().getRequestDispatcher("/jsp/boom.jsp").forward(req, res);
        ...

        // IllegalStateException; cannot redirect after forwarding
        res.sendRedirect("http://www.acme.com/jsp/boomboom.jsp");

        PrintWriter out = res.getWriter();

        // writing to an already-committed stream will not cause an exception,
        // but will not apply these changes to the final output, either
        out.print("Writing here does nothing");

        // IllegalStateException; cannot flush a response's buffer after forwarding the request
        out.flush();
        out.close();
    }
}

In most cases, setting the Content-Length header of a request indicates a developer is interested in
communicating the length of the POST data sent to the server. However, this header should be 0 or a
positive integer.

Example 1: The following code will set an incorrect Content-Length.

  URL url = new URL("http://www.example.com");
  HttpURLConnection huc = (HttpURLConnection)url.openConnection();
  huc.setRequestProperty("Content-Length", "-1000");

Serialization of inner classes lead to serialization of the outer class, therefore possibly leaking information or leading to a runtime error if the outer class is not serializable. As well as this, serializing inner classes may cause platform dependencies since the Java compiler creates synthetic fields in order to implement inner classes, but these are implementation dependent, and may vary from compiler to compiler.

Example 1: The following code allows serialization of an inner class.

  ...
  class User implements Serializable {
    private int accessLevel;
    class Registrator implements Serializable {
      ...
    }
  }

In Example 1, when the inner class Registrator is serialized, it will also serialize the field accessLevel from the outer class User.

A parent class declared the method synchronized, guaranteeing correct behavior when multiple threads access the same instance. All overriding methods should also be declared synchronized, otherwise unexpected behavior may occur.

Example 1: In the following code, the class Foo overrides the class Bar but does not declare the method synchronizedMethod to be synchronized:

public class Bar {
public synchronized void synchronizedMethod() {
    for (int i=0; i<10; i++) System.out.print(i);
    System.out.println();
}
}

public class Foo extends Bar {
public void synchronizedMethod() {
    for (int i=0; i<10; i++) System.out.print(i);
    System.out.println();
}
}

In this case, an instance of Foo could be cast to type Bar. If the same instance is given to two separate threads and synchronizedMethod is executed repeatedly, the behavior will be unpredictable.

The program uses the equals() method to compare an object with null. This comparison will always return false, since the object is not null. (If the object is null, the program will throw a NullPointerException).

Threads spawned by calling pthread_create() from the main() function of the parent process will be terminated prematurely if the parent process finishes execution before any the threads it has spawned without calling pthread_exit(). Calling pthread_exit() guarantees that the parent process will be kept alive until all of its threads have finished execution. Alternatively, the parent process can call pthread_join on all of the child threads and ensure they will complete before the process concludes.

Example 1: The following code uses pthread_create() to create a thread and then exits normally. If the child thread has not completed its execution by the time the main() function returns, then it will be terminated prematurely.

void *Simple(void *threadid)
{
   ...
   pthread_exit(NULL);
}

int main(int argc, char *argv[]) {
   int rc;
   pthread_t pt;
   rc = pthread_create(&pt, NULL, Simple, (void *)t);
   if (rc){
     exit(-1);
   }
}

During deserialization, readObject() acts like a constructor, so object initialization is not complete until this function ends. Therefore when a readObject() function of a Serializable class calls an overridable function, this may provide the overriding method access to the object's state prior to it being fully initialized.

Example 1: The following readObject() function calls a method that can be overridden.

  ...
  private void readObject(final ObjectInputStream ois) throws IOException, ClassNotFoundException {
    checkStream(ois);
    ois.defaultReadObject();
  }

  public void checkStream(ObjectInputStream stream){
    ...
  }

Since the function checkStream() and its enclosing class are not final and public, it means that the function can be overridden, which may mean that an attacker may override the checkStream() function in order to get access to the object during deserialization.

Returning a private readonly list variable from a getter-only property allows the calling code to modify the contents of the list, effectively giving the list write access and contradicting the intentions of the programmer who made it private readonly.

Example 1: The following code contains a list _item which is declared as private readonly.

       class Order
      {
          private readonly List<string> _item = new List<string>();
          public IEnumerable<string> Item { get { return _item; } }
          
          public Order()
          {
              /*class initialize            */
          }

          /*some important function......*/
      }
    

A reentrancy attack occurs when a malicious contract calls back into the calling contract before the first invocation is finished. This happens when the vulnerable contract exposes a function that interacts with external contracts before locally carrying out the effect of the current call. This can lead to the external contract taking over the control flow of the interaction.

If a malicious contract calls the exposed function of a victim that unsafely interacts back with the calling contract, then the attacking contract will receive and process the interaction (via a fallback function for example) and immediately call back the victim's exposed function again in order to enter a call-interact-call loop. This recursive state prevents any further code in the victim to be executed and can lead to partial or total draining of assets or values depending on the nature of the interaction.

The Checks-Effects-Interaction pattern is commonly used in smart contracts to prevent incorrect logic. The pattern designates that the code first check any required conditions, then carry out the related state change (the effect) and finally interact with the external contract in relation to that effect.

Example 1: The following example allows a reentrancy attack by performing a check, interaction, and then effect, not adhering to the Checks-Effects-Interaction pattern.

The code:

1. Checks the balance of the sender (Checks).
2. Sends out Ether to the caller via msg.sender.call.value (Interaction).
3. Performs a state change by decreasing the balance of the sender (Effects).

function withdraw(uint amount) public{
    if (credit[msg.sender] >= amount) {
        require(msg.sender.call.value(amount)());
        credit[msg.sender]-=amount;
    }
}

The code in Example 1 breaks the Checks-Effects-Interaction pattern by doing Checks-Interaction-Effects instead. This can lead to reentrancy if an attacking smart contract receives the Ether in a fallback function and immediately calls back withdraw, creating a recursive situation where Ether is drained because the line of code that decreases the balance (aka the Effect) never gets executed.