0

This is a security hole in java:

// Potential security hole!

static public final Thing[] VALUES = { ... };

A general solution can be

private static final String[] VALUES = { "a", "b" };   
public static final String[] values()
{
    return VALUES.clone();
}

I thought that a cloned array still allows modifying the internal values, why this is considered an accepted solution?

AleWereWolf
  • 13
  • 2
  • 1
    Because now every consumer has a *separate* array of those values, which are themselves immutable. A malicious consumer can only change its own copy. – jonrsharpe Aug 25 '19 at 10:10
  • What happens if the array is an array of objects ? – AleWereWolf Aug 25 '19 at 10:14
  • Then you have to copy deep enough that there's no way for a malicious consumer to alter the values any other consumer sees. – jonrsharpe Aug 25 '19 at 10:15
  • @rustyx It is an issue. Not a security issue, but a design issue. – JB Nizet Aug 25 '19 at 10:24
  • @JBNizet Original comment appears to have been deleted, but whether it is a security issue is context dependent. It is a problem where you have mobile code, for instance, unrestricted Java Deserialization. – Tom Hawtin - tackline Sep 01 '19 at 12:14

1 Answers1

2

It allows modifying the content of the copy of the original array. But the original array isn't modified.

And since each caller of values() receives a separate copy, one caller modifying its copy doesn't affect the other caller.

That said, you should generall prefer collections over arrays. And collections can be made unmodifiable, which avoids doing copies.

public static final List<Thing> VALUES = Collections.unmodifiableList(Arrays.asList(...));

Note however that, whatever the solution you choose, if Thing isn't immutable, anyone can still modify the state of the things, without modifying any array or list.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Is it correct that, if the array contains objects (and no primitive data types as strings) a cloned array is a bad solution ? – AleWereWolf Aug 25 '19 at 10:16
  • No, not particularly. A cloned array takes time to clone, unlike an unmodifiable list which doesn't need cloning. And a cloned array of mutable objects still allows modifying the objects inside the array, but The problem here is not arrays. the problem is shared mutable objects. – JB Nizet Aug 25 '19 at 10:22
  • What kind of problem is the one about "shared mutable objects" ? – AleWereWolf Aug 25 '19 at 10:28
  • The same your question asks about: a caller might modify an object that is supposed to stay constant by not realizing this object is supposed to stay constant and is shared with other pieces of code. – JB Nizet Aug 25 '19 at 10:34