6

Background

In Java 101, we're taught:

A String is immutable.

Yes. Good. Thanks.

Then we get to Java 102 (or perhaps Java 201), and we discover:

A String isn't really immutable: you can change it using reflection.

Ah. Fine. Either quite cute or immeasurably perverse, depending on your perspective.

These things, thus far, have been discussed ad infinitum on Stack Overflow and elsewhere. I am taking this much for granted in framing this question.

What I'm interested to ask is this:

The question

Once we discover that a String isn't really immutable, what are the implications for how we should write our code?

Let me make that more specific in two respects.

JVM Sandboxing

Is it possible for a malicious class to use this trick to break the security of the JVM, and get up to tricks that it shouldn't? Are there places in the JDK, for instance, where a String is returned from a method, and it's safe only on the assumption that it can't be changed?

Here's a promising start:

    String prop = "java.version";
    // retrieve a System property as a String
    String s = System.getProperty(prop);
    System.out.println(s);
    // now mess with it
    Field field = String.class.getDeclaredField("value");  
    field.setAccessible(true);  
    char[] value = (char[])field.get(s);  
    value[0] = 'x';
    //turns out we've changed not just the String we were
    //given but the underlying property too!
    System.out.println(System.getProperty(prop));

What this does is to retrieve a system property from the JVM, which comes back as a String, and then alter the String; the consequence is that the underlying property is changed too. Can this be used to wreak havoc?

I'm not certain. It's worth noting that you have to have the right permissions to perform reflection. Is the security game already up by that point? It allows me here to get round not having permissions to change security properties, but is that to be expected, or is it a problem?

Are there ways of extending this to do something much worse?

Defensive cloning

We're always told that it's a good idea to clone arrays and other objects before passing them to methods that might modify them, unless we want them to be modified. It's just good coding practice. It's your own silly fault if you give someone the only copy of your array and it comes back messed with.

But it looks as though the same argument ought to apply to a String! I have never, ever heard anyone say that we ought to clone a String before passing it to a method someone else has written. But how is this any different, if a String is as mutable as an array?

Argument in favour of defensively cloning strings

If defensive cloning is all about not really knowing what a method might do to the things we pass it, and if what we're passing it is mutable, then we ought to clone it. If the code might be malicious, then it might alter a String; so whenever it's important that the String stay unchanged, we should make sure we send a copy rather than the real thing.

It won't wash to say that untrusted code ought to be run under a security manager if we don't trust it not to do bad things to the String. If that were true for a String, it would be true for an array; but no one ever says that cloning of arrays and other objects is only to be done in cases where you're also locking the code down with a security manager.

Argument against defensively cloning strings

An array might get modified just by sloppy coding; in other words, it might get modified unintentionally. But no one alters a String unintentionally: it can be done only with sneaky tricks that mean the programmer was trying to break immutability.

That makes the two cases different. We really shouldn't be passing an array, even a clone of it, to code that we don't trust on any level. You don't download a library from somewhere dodgy and then think you're OK because you cloned your array. You clone defensively because you think the programmer might be making mistakes or making different assumptions from you about what's allowable with the data you're sending. If you're worried that the code might be modifying Strings behind your back, you really shouldn't be running the code at all. All you achieve by cloning a String is performance overhead.

The answer?

How should we think about this? Can JVM sandboxing be broken using these tricks? And should we code defensively in the light of the mutability of a String, or is this all a red herring?

Community
  • 1
  • 1
chiastic-security
  • 20,430
  • 4
  • 39
  • 67
  • 2
    The answer is: Don't allow the use of reflection in your app, at least not on system objects. (Anyone who allows untrusted code to use reflection in a secure app is a fool.) – Hot Licks Oct 07 '14 at 21:15
  • Java has a security manager and you should either a) prevent developer from performing unsafe operations as per your coding standard, or b) you shouldn't be running code you don't trust without a security manager, or better yet, just don't run code you don't trust whether written internally or not. – Peter Lawrey Oct 07 '14 at 21:38
  • In a "secure" application I can still ask you for your password, credit card details, mother's maiden name etc and some people might type those details in. – Peter Lawrey Oct 07 '14 at 21:40
  • In terms of sandboxing options, you can look at Docker. This gives you controls Java doesn't have like limiting CPU usage etc. – Peter Lawrey Oct 07 '14 at 21:41
  • "You don't download a library from somewhere dodgy and then think you're OK because you cloned your array." The usual security case is that the library has more trust than the code that it is using it. In that case you'll probably want to copy (not usually `clone`) your inputs and outputs of [inappropriately designed] value types. – Tom Hawtin - tackline Oct 08 '14 at 16:08

2 Answers2

4

There is a security setting that can be enabled for your Java program. According to the Javadocs for setAccessible,

First, if there is a security manager, its checkPermission method is called with a ReflectPermission("suppressAccessChecks") permission.

A SecurityException is raised if flag is true but accessibility of this object may not be changed (for example, if this element object is a Constructor object for the class Class).

A SecurityException is raised if this object is a Constructor object for the class java.lang.Class, and flag is true.

So, with a SecurityManager that doesn't allow this check, you can prevent any code from successfully calling setAccessible, preserving the immutability of Strings.

Community
  • 1
  • 1
rgettman
  • 176,041
  • 30
  • 275
  • 357
3

Java has mechanisms in place to prevent this. Specifically, for example, in order to subvert the String, you had to call field.setAccessible(true);, which, for you, works, but if you look at the documentation, that setAccessible(true) call can throw SecurityException.

Java allows you to install, or modify Security Managers that can lock down these reflection APIs. For example, consider Ideone which allows you to run limited Java applications. In large part they do this by limiting the security permissions of the system.

You should read up on the SecuityManager and also the trail document

You should also read through

rolfl
  • 17,539
  • 7
  • 42
  • 76