Why are public static final array a security hole?

Question

Effective java says:

// Potential security hole!

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

Can somebody tell me what is the security hole?

Answer 1

Think it just means the whole public vs private thing. It's supposed to be good practice to have local variables declared private, then using get and set methods, rather than accessing them directly. Makes them a little harder to mess with outside of your program. About it as far as I'm aware.

Answer 2

Declaring static final public fields is usually the hallmark of a class constant. It's perfectly fine for primitive types (ints, doubles etc..), and immutable classes, like strings and java.awt.Color. With arrays, the problem is that even though the array reference is constant, the elements of the array can still be changed, and as it's a field, changes are unguarded, uncontrolled, and usually unwelcome.

To combat this, the visibility of the array field can be restricted to private or package private, so you have a smaller body of code to consider when looking for suspicious modification. Alternatively, and often better, is to do away with the array together and use a 'List', or other appropriate collection type. By using a collection, you control if updates are allowed, since all updates go through methods. You can prevent updates by wrapping your collection using Collections.unmodifiableList(). But beware that even though the collection is immutable, you must also be sure that the types stored in it are also immutable, or the risk of unsolicited changes on a supposed constant will reappear.

Answer 3

Note that a nonzero-length array is always mutable, so it is wrong for a class to have a public static final array field, or an accessor that returns such a field. If a class has such a field or accessor, clients will be able to modify the contents of the array.

-- Effective Java, 2nd Ed. (page 70)

Answer 4

In this declaration, a client can modify Thing[0], Thing[1], etc (i.e. elements in the array).

Answer 5

An outside class can modify the contents of the array, which is probably not what you want users of your class to do (you want them to do it through a method). It's sounds like the author meant it violates encapsulation, and not security.

I guess someone declaring this line could think other classes can't modify the array contents since it's marked as final, but this is not true, final only stops you from re-assigning the attribute.

Answer 6

Because, final keyword assures only reference value (assume it as memory location for example) but not the content in it.

Answer 7

To understand why this is a potential security hole and not just poor encapsulation, consider the following example:

public class SafeSites {
    // a trusted class with permission to create network connections
    public static final String[] ALLOWED_URLS = new String[] {
        "http://amazon.com", "http://cnn.com"};

    // this method allows untrusted code to connect to allowed sites (only)
    public static void doRequest(String url) {
        for (String allowed : ALLOWED_URLS) {
            if (url.equals(allowed)) {
                 // send a request ...
            }
        }
    }
}

public class Untrusted {
     // An untrusted class that is executed in a security sandbox.

     public void naughtyBoy() {
         SafeSites.ALLOWED_URLS[0] = "http://myporn.com";
         SafeSites.doRequest("http://myporn.com");
     }
}

As you can see, the mistaken use of a final array means that untrusted code can subvert the restriction that the trusted code / sandbox is trying to impose. In this case, this is clearly a security issue.

If your code is not part of a security critical application, then you could ignore this issue. But IMO this is a bad idea. At some point in the future you (or someone else) might reuse your code in a context where security is a concern. At any rate, this is why the author calls public final arrays a security issue.