Here’s what happens when good code meets bad people.
December 21, 2020 | Download a PDF of this article
More quiz questions available here
If you have worked on our quiz questions in the past, you know none of them is easy. They model the difficult questions from certification examinations. We write questions for the certification exams, and we intend that the same rules apply: Take words at their face value and trust that the questions are not intended to deceive you but to straightforwardly test your knowledge of the ins and outs of the language.
The objective of this Java SE 11 quiz is to develop code that mitigates security threats, such as denial of service attacks or code injection.
Here’s your secret mission: You are developing a highly loaded, multithreaded stock-quote analysis application. During a security audit, you were advised to make a defensive copy of stock-quote data as you receive it for analysis, isolating the calculations from any modifications made to the original input data.
Given this StockQuote
class:
class StockQuote {
public StockQuote(String n, LocalDateTime t, Double p) {
name = n;
time = t;
price = p;
}
private String name;
private LocalDateTime time;
private Double price;
// getters and setters hidden for brevity
}
and this method, which should protect itself by making a copy:
public void analyzeQuotes(ArrayList<StockQuote> sqList) {
List<StockQuote> sqListProtected = … // make a defensive copy
}
Which code line (if any) can protect you from malicious modifications made to the stock-quote data during the processing? Choose one.
A. List<StockQuote>)sqList.clone();
B. new ArrayList<StockQuote>(sqList);
C.
(List)sqList.stream()
.map(s -> s.clone())
.collect(Collectors.toList());
D.
sqList.stream()
.map(s -> s.clone())
.collect(Collectors.toCollection(ArrayList::new));
E. None of the above
Answer. In option A, the clone()
method of ArrayList
will perform “shallow” cloning. This means that the result will be a new list structure, but the values in that list will be references to the same objects as were referred to by the original list. Because of this, any changes made to the items in the original list will be visible through the new list. The cloning operation will protect only against additions, deletions, or changes of sequential order made to the original list.
This observation about shallow cloning applies not only to ArrayList
but also to all objects that use the default implementation of Object.clone()
and contain references to other mutable objects. If an object contains only primitives or references to immutable instances such as String
, the cloned copy will be protected against changes made to the original list. From this you can determine that option A is incorrect.
If you were unsure whether the clone()
method performs a shallow or deep copy, there’s another hint that could help to determine that the code in option A doesn’t work. Because the StockQuote
class does not implement the java.lang.Cloneable
marker interface, it cannot be cloned. Therefore, if ArrayList
attempted a deep clone—including duplicating its member objects—the attempt would throw a java.lang.CloneNotSupportedException
.
Option B is essentially equivalent to option A. Passing a collection to the ArrayList
constructor will create a new list structure containing the same reference values, in the same order, as in the original collection. But the StockQuote
objects are shared between both, and changes made to any of those objects via sqList
will result in changes visible through the new list. Because of this, option B is also incorrect.
It’s worth noting that the approach demonstrated in option B can be used to produce a different type of collection: perhaps changing between an unmodifiable collection (that is, one that disallows adding or removing elements or changing their order) and a fully modifiable collection.
There are three problems with option C:
- The
Object.clone()
method is declared as protected
, so it can be called only from a subclass using the syntax super.clone()
or this.clone()
but not from some arbitrary object reference in the manner of s.clone()
shown in the code.
- Even if the
s.clone()
code were valid, the Object.clone
method throws a checked exception java.lang.CloneNotSupportedException
. But the argument to the map
method must be a java.util.function.Function<A,B>
implementation, and the apply()
method of that functional interface does not permit checked exceptions. Hence, the code would still not compile.
- Even if the first two problems didn’t exist and the code could compile, it would still fail to execute. This is because, as was noted earlier, the
StockQuote
class does not permit cloning, so the clone
method would definitely fail and throw a CloneNotSupportedException
. This exception would crash the stream operation.
In view of all these problems, it’s clear that option C is incorrect.
Option D has all the same problems as with option C, so option D is also incorrect.
In fact, option D has another difficulty, which is that the Object.clone()
method returns Object
. So even if all three problems from option C were somehow avoided, the collect
operation would result in an ArrayList<Object>
, and that’s not assignment-compatible with the variable in the method, which is of type List<StockQuote>
.
Since none of the options A–D provide a required solution, this makes option E correct.
Before leaving this quiz, let’s consider how this code could be protected. The fields in the StockQuote
class are of type String
, LocalDateTime
, and Double
. Each of these are classes that create immutable instances, so it’s safe for two StockQuote
objects to share a reference to the same attributes. So, one approach is to create copies of the StockQuote
objects and a new list to contain those copies.
You could pursue a solution that would work when used to build on the code of option C (note this is not the only possible solution). That code is built on the assumption that the StockQuote
items can be cloned in the map
method. So first you must ensure that StockQuote
conforms to that expectation. This requires two steps:
- The class implements the
Cloneable
interface (this grants permission to the JVM to perform cloning)
- You provide a useful implementation of the
clone
method
Step 1 is simply achieved like this:
class StockQuote implements Cloneable {
At this point, it’s possible to clone the StockQuote
objects successfully. However StockQuote
inherits the default implementation of clone()
from Object
, and this throws CloneNotSupportedException
, which is a checked exception.
Checked exceptions are incompatible with Java’s functional interfaces. In this code, if the exception arises, it is essentially unrecoverable, so create an overriding clone()
implementation and wrap the exception in a RuntimeException
.
There are three further steps for how you implement the clone
method:
- The default
clone
method is protected
. However, you want to be able to call this from inside the map
method. Consequently you must make this public
.
- The default
clone
method makes a shallow copy; that is, it copies all the member variables. In this situation, this is acceptable since all the fields are immutable objects (String
, LocalDateTime
, and Double
). So, the method will delegate to super.clone()
to do the bulk of its work.
- The default
clone
method is declared to return Object
, but Java permits an overriding method to provide a covariant return. That means that you can return a more specific type: something assignment-compatible with the overridden method’s declared type. In your code, the sensible choice is to return the StockOption
type. After all, that’s what the object will actually be. Doing so avoids creating a raw List
, and it allows you to remove the raw cast to List
in option C. It also allows option D to work.
After all that, your implementation of clone()
looks like this.
@Override
public StockQuote clone() {
try {
return (StockQuote)super.clone();
} catch (CloneNotSupportedException e) {
throw new RuntimeException(e);
}
}
Unfortunately, even now, you’re still not done! You still cannot expect objects that are presented to you as StockQuote
instances to reliably clone as intended. Certainly, actual instances of the StockQuote
class presented here will clone as you have defined; that part is correct.
However, you have not precluded an attacker from creating a subclass of StockQuote
. Such a class could override the clone()
method so that it simply returns itself (the this
reference). In so doing, it would completely bypass all your good intentions. So, your last step should be to make the class itself, or at least the clone()
method, final
.
By the way, control of subclassing is a focus of the sealed classes feature that’s previewed in Java 15. This allows the creator of a class to explicitly define the permitted subclasses, so an inheritance tree can exist, but it prevents arbitrary additions to that tree.
[You can read about sealed classes in “Inside the language: Sealed types,” by Ben Evans. —Ed.]
In closing, we must acknowledge that the exact nature of the security concerns isn’t clearly stated in the question, and other excellent approaches to this are possible. But this was meant to be a discussion, not a full analysis (which would require a book!). So we’ll wrap this up here.
Conclusion: The correct answer is option E.