16
votes

Does using reflection to scrub a String make using String as safe as using char[] for passwords?

From a security aspect, it is generally considered best practice to use char[] for storing/passing passwords, because one can zero-out its contents as soon as possible in code, which may be significantly before garbage collection cleans it up and the memory is reused (wiping all trace), limiting the window of time for a memory attack.

However, char[] is not as convenient as String, so it would be handy if one could "scrub" a String if needed, thus making String as safe as char[].

Below is a method that uses reflection to zero-out the fields of String.

Is this method "OK", and does it achieve the goal of making String as safe as char[] for passwords?

public static void scrub(String str) throws NoSuchFieldException, IllegalAccessException {
    Field valueField = String.class.getDeclaredField("value");
    Field offsetField = String.class.getDeclaredField("offset");
    Field countField = String.class.getDeclaredField("count");
    Field hashField = String.class.getDeclaredField("hash");
    valueField.setAccessible(true);
    offsetField.setAccessible(true);
    countField.setAccessible(true);
    hashField.setAccessible(true);
    char[] value = (char[]) valueField.get(str);
    // overwrite the relevant array contents with null chars
    Arrays.fill(value, offsetField.getInt(str), countField.getInt(str), '\0');
    countField.set(str, 0); // scrub password length too
    hashField.set(str, 0); // the hash could be used to crack a password
    valueField.setAccessible(false);
    offsetField.setAccessible(false);
    countField.setAccessible(false);
    hashField.setAccessible(false);
}

Here's a simple test:

String str = "password";
scrub(str);
System.out.println('"' + str + '"');

Output:

""

Note: You may assume that passwords are not String constants and thus calling this method will have no adverse effect on interned Strings.

Also, I have left the method is a fairly "raw" state for simplicity's sake. If I were to use it, I would not declare exceptions thrown (try/catch/ignoring them) and refactor repeated code.

2
Well, it seems to zero out the chars, but isn't that quite dependent on the JVM implementation?fge
memory snooping? that's a fantasy attack. why not worry about something more realistic, like NSA snooping your network traffic.ZhongYu
Screams "undefined behavior" at me. Good luck with dragons, velociraptors and black holes :)CodesInChaos
Your code depends on the implementation of the String class. And it happens that the Oracle JDK/openJDK from version 7u6 have a different implementation and your scrub method would throw an exception.assylias
A good implementation of String wont have offset. You ought to be a bit more robust (not something that really happens with most reflection code). / "Memory snooping" isn't an entirely unrealistic attack, particularly when you consider swapping/hibernation may place data on physical medium for a surprising length of time.Tom Hawtin - tackline

2 Answers

7
votes

There are two potential safety concerns:

  1. The String may share its backing array with other Strings; e.g. if the String was created by calling substring on a larger String. So when you zero the entire value array you could be overwriting the state of other strings ... that don't contain passwords.

    The cure is to only zero the part of the backing array that is used by the password string.

  2. The JLS (17.5.3) warns that the effects of using reflection to change final variables is undefined.

    However, the context for this is the Java Memory Model, and the fact that the compiler is allowed to aggressively cache final variables. In this case:

    • you would expect the String to be thread-confined, and

    • you shouldn't be using any of those variables again.

I wouldn't expect either of these to be real problems ... modulo fixing the over-aggressive zeroing of value.


But the real concern is Velociraptors. :-)


I'm puzzled that you would actually bothering to zap passwords like this. When you think about it, what you are protecting against is the possibility that someone can read process memory ... or a core dump or swap file ... to retrieve passwords. But if someone can do that, your system security has to have already been compromised ... cos' those things most likely require root access (or equivalent). And if they have root access they can "debug" your program and catch the passwords before your application zaps them.

1
votes

One argument I have against String is that it's just too easy to inadvertently make a copy. Using strings safely is possible in theory, but the whole library ecosystem is based on the assumption that it's perfectly OK to copy strings. In the end, considering all the restrictions, strings may not be as convenient for this use case as they generally are.