4 ms·

For locally generated strings, it's not a concern. For String.intern, this is actually a very serious bug that should be treated as a security vulnerability.

gizmo686 3y ago
At best it is only a security vulnerability if you are running different trust domains within your process. That is something Java supports, but most Java code does not attempt to take advantage of it. Most Java programs have 100% of there code being allowed to use reflection to simply hack the program however they please, including corrupting the intern pool. But still, for the few people who actually trust Java's isolation features and use it, possible bypasses are a concern. Having said that, I don't see how you can turn this into an attack on String.intern. The guarentees about interning are as follows: > When the intern method is invoked, if the pool already contains a string equal to this String object as determined by the equals(Object) method, then the string from the pool is returned. Otherwise, this String object is added to the pool and a reference to this String object is returned. > It follows that for any two strings s and t, s.intern() == t.intern() is true if and only if s.equals(t) is true. If you pollute the pool with a "broken" String, all that would happen is that that String would not be returned in place of a logically equivalent correct String, and vice-versa.
- hashmash 3y ago
  If a security sensitive operation was (for whatever reason) calling startsWith (as shown in the blog), then it might return an incorrect result. I'm merely speculating that an exploit is possible, and I'm not aware of any code that would be affected. Regardless, the fix is quite simple. The public String.intern method just needs to validate the encoding first. This will have little impact on performance because the String.intern method is rarely called.
  - dundarious 3y ago
    Their point, which I agree with, is that shared memory concurrency requires correct code, and cooperative synchronization between threads, in order to avoid data races; and if your security model is to run untrusted or unverified code with shared memory concurrency, then you cannot ensure correct code or synchronization, and you cannot guarantee against data races. It's perfectly fine to code String.intern() defensively against such dodgy String-s, but in any sizable program with shared memory concurrency and untrusted or unverified code, there will be millions of other potential data races. The only sensible choice would be to not use such a mix.

gizmo686 3y ago

At best it is only a security vulnerability if you are running different trust domains within your process. That is something Java supports, but most Java code does not attempt to take advantage of it. Most Java programs have 100% of there code being allowed to use reflection to simply hack the program however they please, including corrupting the intern pool. But still, for the few people who actually trust Java's isolation features and use it, possible bypasses are a concern. Having said that, I don't see how you can turn this into an attack on String.intern. The guarentees about interning are as follows: > When the intern method is invoked, if the pool already contains a string equal to this String object as determined by the equals(Object) method, then the string from the pool is returned. Otherwise, this String object is added to the pool and a reference to this String object is returned. > It follows that for any two strings s and t, s.intern() == t.intern() is true if and only if s.equals(t) is true. If you pollute the pool with a "broken" String, all that would happen is that that String would not be returned in place of a logically equivalent correct String, and vice-versa.

hashmash 3y ago
If a security sensitive operation was (for whatever reason) calling startsWith (as shown in the blog), then it might return an incorrect result. I'm merely speculating that an exploit is possible, and I'm not aware of any code that would be affected. Regardless, the fix is quite simple. The public String.intern method just needs to validate the encoding first. This will have little impact on performance because the String.intern method is rarely called.
- dundarious 3y ago
  Their point, which I agree with, is that shared memory concurrency requires correct code, and cooperative synchronization between threads, in order to avoid data races; and if your security model is to run untrusted or unverified code with shared memory concurrency, then you cannot ensure correct code or synchronization, and you cannot guarantee against data races. It's perfectly fine to code String.intern() defensively against such dodgy String-s, but in any sizable program with shared memory concurrency and untrusted or unverified code, there will be millions of other potential data races. The only sensible choice would be to not use such a mix.