Search code examples
javastringcode-injection

Does Java String.format() prevent String injection?

I have been preparing for technical interview. So in one thing I am just not sure. If I write for example

try {
...
} catch (InterruptedException ie) {
    throw new IllegalStateException(String.format("Player [%s]: failed to reply message [%s]"), ie);
}

Does this prevent already String injection? or Do I have to write like following:

String errorMsg = String.format("Player [%s]: failed to reply message [%s]");
throw new IllegalStateException(errorMsg, ie);
Solution

  • There is no difference between the two snippets. Neither protects against 'string injection'.

    There are only 4 non-boneheaded mitigations against string injection attacks:

    1. Ensure that where-ever the strings end up, it is impossible for this to be a security issue in any way or form. For example, if your data is going to a binary file where all system operators are aware the contents are straight from the web, it doesn't matter what's uploaded.

    2. Do not render the string at all. Throwing the baby out with the bathwater.

    3. Use a whitelist. If the string consists solely of these allowed things, allow it. By default, do not allow it.

    4. Use escapers.

    5. Honourable mention for the concept of blacklisting: Have a list of known-malicious stuff, and allow all strings unless they contain something on the blacklist. This is bone headed and should never be used. For example, if you scan incoming data for <script>, you messed up. Don't do this. It doesn't work. Blacklists are trivially bypassed. Whitelists are what you're looking for.

    The vastly most common strategy is the 4th: Escapers. For example, when you have a web server that takes in a username and a user's telephone number and a user's full name, and then renders all this information back out on their public website, then:

    • The phone number should be mitigated using the whitelist strategy. A single +, digits in the range 0-9, spaces, dashes, and nothing else. If that's what the input is like, allow it. Otherwise don't.

    • The user's real name should be mitigated with escaping: Take the data as provided and inject it verbatim into your database, but treat this data as tainted in all interactions with that data: For example, when rendering that public page, the 'full name' string needs to be washed through an HTML escaper which e.g. replaces all < with &lt;.

    Your code doesn't do any of these 4 things (either version of it).

    In general it is an extremely bad idea to consider the string returned by an exception's .getMessage() to be already 'safe' (escaped / passed the whitelist verifier). Instead, the code that invokes .getMessage() needs to apply one of the 4 mitigations as explained above.