Do not use the
null value in any instance where an object is required, including the following cases:
- Calling the instance method of a null object
- Accessing or modifying the field of a null object
- Taking the length of
nullas if it were an array
- Accessing or modifying the elements of
nullas if it were an array
- Throwing
nullas if it were a
Throwablevalue
Using a
null in cases where an object is required results in a
NullPointerException being thrown, which interrupts execution of the program or thread. Code conforming to this coding standard will consequently terminate because ERR08-J. Do not catch NullPointerException or any of its ancestors requires that
NullPointerException is not caught.
Noncompliant Code Example
This noncompliant example shows a bug in Tomcat version 4.1.24, initially discovered by Reasoning [Reasoning 2003]. The
cardinality() method was designed to return the number of occurrences of object
obj in collection
col. One valid use of the
cardinality() method is to determine how many objects in the collection are null. However, because membership in the collection is checked using the expression
obj.equals(elt), a null pointer dereference is guaranteed whenever
obj is null and
elt is not null.
public static int cardinality(Object obj, final Collection<?> col) { int count = 0; if (col == null) { return count; } Iterator<?> it = col.iterator(); while (it.hasNext()) { Object elt = it.next(); if ((null == obj && null == elt) || obj.equals(elt)) { // Null pointer dereference count++; } } return count; }
Compliant Solution
This compliant solution eliminates the null pointer dereference by adding an explicit check:
public static int cardinality(Object obj, final Collection col) { int count = 0; if (col == null) { return count; } Iterator it = col.iterator(); while (it.hasNext()) { Object elt = it.next(); if ((null == obj && null == elt) || (null != obj && obj.equals(elt))) { count++; } } return count; }
Noncompliant Code Example
This noncompliant code example defines an
isProperName
() method that returns true if the specified
String argument is a valid name (two capitalized words separated by one or more spaces):
public boolean isProperName(String s) { String names[] = s.split(" "); if (names.length != 2) { return false; } return (isCapitalized(names[0]) && isCapitalized(names[1])); }
Method
isProperName
() is noncompliant because it may be called with a null argument, resulting in a null pointer dereference.
Compliant Solution (Wrapped Method)
This compliant solution includes the same
isProperName() method implementation as the previous noncompliant example, but it is now a private method with only one caller in its containing class.
public class Foo { private boolean isProperName(String s) { String names[] = s.split(" "); if (names.length != 2) { return false; } return (isCapitalized(names[0]) && isCapitalized(names[1])); } public boolean testString(String s) { if (s == null) return false; else return isProperName(s); } }
The calling method,
testString(), guarantees that
isProperName
() is always called with a valid string reference. As a result, the class conforms with this rule even though a public
isProperName() method would not. Guarantees of this sort can be used to eliminate null pointer dereferences.
Compliant Solution (Optional Type)
This compliant solution uses an
Optional String instead of a
String object that may be null. The
Optional class (
java.util.Optional [API 2014]) was introduced in Java 8 and can be used to mitigate against null pointer dereferences.
public boolean isProperName(Optional<String> os) { String names[] = os.orElse("").split(" "); return (names.length != 2) ? false : (isCapitalized(names[0]) && isCapitalized(names[1])); }
The
Optional class contains methods that can be used to make programs shorter and more intuitive [Urma 2014].
Exceptions
EXP01-J-EX0: A method may dereference an object-typed parameter without guarantee that it is a valid object reference provided that the method documents that it (potentially) throws a
NullPointerException, either via the
throws clause of the method or in the method comments. However, this exception should be relied on sparingly.
Risk Assessment
Dereferencing a null pointer can lead to a denial of service. In multithreaded programs, null pointer dereferences can violate cache coherency policies and can cause resource leaks.
Automated Detection
Null pointer dereferences can happen in path-dependent ways. Limitations of automatic detection tools can require manual inspection of code [Hovemeyer 2007] to detect instances of null pointer dereferences. Annotations for method parameters that must be non-null can reduce the need for manual inspection by assisting automated null pointer dereference detection; use of these annotations is strongly encouraged.
Related Vulnerabilities
Java Web Start applications and applets particular to JDK version 1.6, prior to update 4, were affected by a bug that had some noteworthy security consequences. In some isolated cases, the application or applet's attempt to establish an HTTPS connection with a server generated a
NullPointerException [SDN 2008]. The resulting failure to establish a secure HTTPS connection with the server caused a denial of service. Clients were temporarily forced to use an insecure HTTP channel for data exchange.
Related Guidelines
Android Implementation Details
Android applications are more sensitive to
NullPointerException because of the constraint of the limited mobile device memory. Static members or members of an Activity may become null when memory runs out.
25 Comments
Klaus Havelund
I have four comments to this rule:
1) Although a desirable rule, this is a rule that seems very cumbersome to enforce in general. If all dereferences in Java were to be guarded by a null test, then code would be cluttered with all these tests.
2) One could of course just require that if a static analyzer cannot determine whether a pointer is null, then a test should be performed. That is perhaps more reasonable.
3) However, what would one do when detecting a null? throw a NullPointerException? .. in which case one can just as well just let the JVM do it.
4) The problem is obvious, just like one should not divide by zero or that one should not index outside array bounds.
Dhruv Mohindra
Thank you for your comments.
1) It does appear to be slightly hard to enforce. There are some cases where there is an obvious bug which can be detected. For example -
There is the problem of checking for
null, clutter etc. and we can't do much about it. Guidelines like MET10-J. For methods that return an array or collection prefer returning an empty array or collection over a null value and OBJ14-J. Encapsulate the absence of an object by using a Null Object provide a starting point.
2) I think this is a quality of implementation issue. If a tool cannot determine whether a pointer is
null, it is less likely that a programmer will even realize there is a problem (and install tests). However, if a tool determines there is a problem, the developer must fix it or eliminate any false positives which might be a reasonable requirement. This guideline may not require the programmer to check for
nullunder all cases then.
3) This seems to go into runtime checking. Current tools like Findbugs evaluate likely branches and check code accordingly. The idea is to avoid obvious null dereferences before the code is run.
4) The problem is about fixing errors in your code, just like other problems that cause runtime exceptions. Just that its frequency of occurrence is higher.
In summary, the focus is on detecting obvious dereferences more than recommending checking for null on every instance. Does this address your comment?
EDIT: I also think annotations like @NonNull will help here.
Dhruv Mohindra
Regarding point 3) above I saw some JDK code (
String.java):
That looks strange to me, given your point. It seems checking and throwing an NPE explicitly may be more costly performance wise than letting the JVM do it for you.
Masaki Kubo
A comment to the risk assessment section: I'd like to see some reference that explains the relationship between cache coherency policy and null dereference in Java. Also how resource leak occurs.
Daniel BÃ¶gner
To put my two cents in, partially compatible to was already has been written:
This rule looks to me like: "Don't produce bugs!" Maby sometimes it is a lack of specifications, too.
If there is a specification of an algorithm it should clearly tell, if the parameters of the algorithm are allowed to refer to null or not.
Now, what is exactly causing the DOS? The occuring NullPointerExceptions? Calling code that includes bugs and may throw arbitrary exceptions?
Until this question is answered I would set this rule in question.
David Svoboda
I'll agree with you regarding specifications...I've seen a lot of APIs that are not properly specified.
The DOS is caused by the null pointer exception interrupting the program, and probably causing it to abort. The Java community has decided against letting NPEs be thrown and caught later.
David Svoboda
I just saw your comment on rule ERR08-J. Do not catch NullPointerException or any of its ancestors. The comment notes that one of the compliant solutions in that rule violates this rule, and any responses to that comment IMO belong here rather than there.
David Svoboda
Your comment raised a valid point: this rule would appear to require null checks on all objects passed to methods (lest a method dereference a null object parameter). Which we clearly don't support, as noted in the compliant code examples from ERR08-J.
So I have added EXP01-EX0, an exception to this rule, which should address those code examples, and your comment. What do you think?
Yitzhak Mandelbaum
Daniel,
While I see your point, I'm not sure that I agree with it. Consider the following (contrived) code:
What should be the annotation/comments for obj1? Well, it has a functional dependency on argument objIsNotNull. So, it really needs a conditional annotation. While this particular example is contrived, I believe this kind of pattern arises frequently. Specifically, the nullness of a given parameter might depend (in a complex manner) on other state, perhaps even global state.
How does this pattern relate to the rule, as stated? Well, currently, the rule just says:
which does not rule out the given pattern. Of course, we could argue that it should, but I think that would be a hard sell and would certainly contradict a lot of existing code.
Yitzhak Mandelbaum
The exception EXP01-EX0 doesn't seem in line with David's earlier comment:
nor with ERR08-J. Specifically, what value does documenting the potential for a NullPointerException have, if you're not supposed to catch it?
Yitzhak Mandelbaum
After discussion with David S., we decided to leave this exception despite the above objections, as a concession to legacy code. However, it should be invoked with care. We also intend to revisit this decision in the future based on experience with checking codebases.
Dhruv Mohindra
I don't think one should document an NPE in the form of javadocs for the method (or else soon you will have everyone doing it for all methods. Moreover it is not a checked exception.). It's better to leave a comment in the method itself. Again, I am unsure how a comment would actually solve the problem of culling NPEs.
Yitzhak Mandelbaum
Dhruv,
Regarding your overall sentiment: I agree in principle and hence the intent to revisit this decision.
Regarding the comment placement: can you expand on why you see it preferable to place outside the javadoc comments? I would think it preferable to warn clients of the unsafety rather than to tuck it away as a "TODO" style comment in the code.
Dean Sutherland
One plausible alternative approach would be adding the @NotNull annotation to the parameter that won't be checked for null. Benefits include:
Note that simply adding the annotation is insufficient from a security point of view. For security, you'd need a successful analysis that proves that all callers pass non-null actuals.
I'd vote for requiring the annotation as part of claiming the exception.
Robert Seacord
This seems like a reasonable requirement to me as well.
Dhruv Mohindra
I don't think I have ever seen an NPE documented in @throws. I just searched and came across this quote:
from this Oracle link.
IMO, NPEs should be avoided and the client would probably expect the called code to not throw an NPE too. As we are talking about rules, a TODO comment would not eliminate the problem. Checking for null and taking appropriate action may be recommended. In fact an appropriate action can be returning null which is better and can be documented in @returns. If that smells, return an empty map or list instead of null. Throwing an NPE explicitly is not of much use.
Thomas Hawtin
This rule is 100% wrong. Null-tolerance is a common cause of RCEs in the JRE. If the result of a bug has been detected, get the hell out of there by throwing an unchecked exception.
(Interestingly from CWE-476 "In very rare circumstances and environments, code execution is possible." The only case of dereferencing from a null that caused an RCE that I can remember was a Flash bug that used a null as the base of an array. If the code was explicitly null-intolerant, then it would not have been an RCE (in fairness, there are many other ways the RCE could have been avoided).)
Yitzhak Mandelbaum
Thomas,
Sorry, I don't quite follow your point: the rule says nothing about null tolerance. It simply says not to dereference null pointers. You are more than welcome (and I daresay encouraged) to throw a different exception if a null pointer is encountered when one was not expected.
This rule is no worse than any other rule instructing you not to misuse an API. And, qualitatively, it is even more important because NPEs are so pervasive (see Tony Hoares "Billion Dollar Mistake" talk). Hence the wealth of academic and industrial work related to statically rooting out (potential) null derefs. If you can suggest an alternative approach that would accomplish the same goal, then please suggest it. Pervasive use of the relevant annotations would be one approach, for example (if somewhat limiting as per my earlier comment to Daniel).
Regarding RCE: again, the rule says nothing related to RCEs. It identifies the problem as DoS, which, while admittedly far less problematic than RCE, is still a serious problem.
Thomas Hawtin
The body of the article is talking about null-tolerance. If you don't throw an exception when you encounter an [erroneous] null, what else is there?
In Java SE 7 you are encouraged to use Objects.requiresNonNull, which throws an NPE.
The Java tradition is to always check arguments, throw an exception in the event of an error. In this way bugs are caught quickly and execution with erroneous data is stopped. Following the advice of this rule, some kind of meaning is given to nulls and illegal execution continues in some cases causing RCEs. Something should probably be said about RCEs.
A good implementation of "cardinality" is the OpenJDK implementation of Collections.frequency (in the Java library from J2SE 5.0). It correctly NPEs when given a null Collection (could have been Iterable, but that would be inconsistent). As is common, the loop is duplicated for null and non-null comparisons. If you want a null-tolerant equals, Objects.equals is provided from Java SE 7. Perhaps the easiest to read null-tolerant equals expression is `a==null ? b==null : a.equals(b)`. Generally the best implementation is `a == b || (a !?= null && a.equals(b))`, I suggest that is put in its own method if using old versions of Java.
Yitzhak Mandelbaum
Thomas,
Sorry, I don't see how you get that from what's written. We never advise not to throw an exception when you encounter an *erroneous* null. In both examples,
nullis a perfectly legal input that is handled as such. Can you point us to the specific place(s) in the text which you see as being null tolerant? I would be helpful so we can clarify the code/text.
That said, perhaps we view
nulldifferently. Are you arguing that
nullshould never be allowed and therefore always be considered erroneous? The key point that we're aiming for is that you not rely on the JVM to check your null errors for you. Instead, you should proactively build your code to either avoid them entirely, or handle them correctly when they are expected to appear. This approach is independent of issues of null tolerance, which I understand to mean handling null when it is not expected to appear.
Yitzhak Mandelbaum
Regarding RCEs: please give an example (CVE, etc.) of an RCE in Java code which did not exploit a bug in the underlying JVM or a flawed security policy. My general impression is that Java's design guards against the traditional RCEs seen in C and C++ code (and other weakly typed languages).
David Svoboda
Thomas, thanks for reminding me of the Mark Dowd flash vulnerability. That issue was specific to C. There are other vulnerabilities associated with null pointers, mostly due to unusual circumstances where dereferencing a null pointer in C doesn't actually terminate the program. Remember, the C standard does not mandate that dereferencing null quits the program, it's merely a convention many platforms follow. None of this applies to Java.
I suspect a lot of code can be excused by EXP01-EX0. You might argue that the NCCEs here could be excused if only they documented that they might throw NPE.
The danger of this rule is twofold: that a user might be tripped up by an unexpected NPE and not know how to handle it in the field. And a developer might be tripped up in testing by an unexpected NPE. Developers are comfortable with NPEs thrown by methods they call, and are used to ensuring they don't send NULL to the wrong method. The real issue is if a NPE occurs in some method that they did not invoke. EG they call a() which calls b(), which...which calls z() which throws an NPE.
There are two solutions to this problem: Either z() does something else, such as throwing YouProvidedThisBadInputException (eg something more specific/helpful to the developer than NPE). Or all the methods are documented wrt null-tolerance, so you can see which method passed invalid data.
Yitzhak Mandelbaum
I spent some time reading up on this issue and have come to the conclusion that this rule is misguided and should be removed. Java is broken. This rule will not fix it, and has a high likelihood of making things worse. A few example unintended consequences:
See this excellent article Null Dereference Analysis in Practice, by Ayewah and Pugh (of findbugs fame), for a detailed discussion.
Thomas Hawtin
Interesting that the single-argument doPrivileged has been added to the bibliography. The two-argument form is more interesting with regards to nulls. "If the context is
null, then no additional restriction is applied." That is to say, if there's a way to execute the code without having initialised the acc variable, you then give all permissions rather than restricted permissions. It really helps code understandably if you do a null check every time you use these methods. That doesn't stop people wanting to remove the "code clutter" to create vulnerabilities.
Also of note in Java SE 7 is java.util.Collections.requireNonNull. Although rather verbose and has (formerly) non-standard capitalisation, it does allow a null check to be written without necessarily adding a line of code. I note the bilbliography still points to Java SE 6 - that will no longer be supported shortly.
Dean Sutherland
We definitely need to add a CS that uses methods from java.util.Objects (such as requireNonNull). The null-tolerant utility methods found there would clearly help with this issue. Also, an appropriate caution about both forms of doPrivileged is clearly in order.
I'll work on both of these later today.