Arbitrary stack overflow in SSLSocketChannel2.read()

[Erhannis]

Every so often I get a StackOverflowError in SSLSocketChannel2.read(). I don't see any correlation between when they happen; seems random. Here's my stack trace:

0 java.lang.StackOverflowError
1 at org.apache.harmony.xnet.provider.jsse.SSLEngineImpl.unwrap(SSLEngineImpl.java:412)
2 at javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:385)
3 at org.java_websocket.SSLSocketChannel2.unwrap(SSLSocketChannel2.java:150)
4 at org.java_websocket.SSLSocketChannel2.readRemaining(SSLSocketChannel2.java:254)
5 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:220)
6 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
7 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
8 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
........
254 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
255 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
256 at org.java_websocket.SSLSocketChannel2.read(SSLSocketChannel2.java:241)
257 at org.java_websocket.SocketChannelIOHelper.read(SocketChannelIOHelper.java:12)
258 at org.java_websocket.client.WebSocketClient.interruptableRun(WebSocketClient.java:229)
259 at org.java_websocket.client.WebSocketClient.run(WebSocketClient.java:188)
260 at java.lang.Thread.run(Thread.java:1019)

As near as I can tell, it looks like for some reason no bytes are getting transferred from inData to dst, repeatedly. Not sure why, though.

[Davidiusdadi]

Thank you for reporting the problem.
This is a bug of the websocket client.

The bug appears to happen when more network data is received that the sslengine can decode in one chunk.
Because of a sloppy(buggy) condition the SSLSocketChannel2 trys to decode the remaining incomplete chunk of encrypted network data over and over again ignoring any further available network data.

I will push a fix for that very soon.

[Erhannis]

Great; thanks! Except...it doesn't seem to work. It seems like you were right about the cause, though; I get the error consistently when I send a large message.

[Davidiusdadi]

have you pulled from my repo(https://github.com/Davidiusdadi/Java-WebSocket
)?

2013/8/2 Erhannis notifications@github.com

Great; thanks! Except...it doesn't seem to work. It seems like you were
right about the cause, though; I get the error consistently when I send a
large message.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/190#issuecomment-21974310
.

[Erhannis]

Yep. I tested it merged with the TooTallNate repo, as well as on its own, but either way it still overflowed.

[Davidiusdadi]

Your callstack definitely matches the code prior to the fixes. It seems you
pulled form the wrong source. Please make sure that you pull these changes
form Davidiusdadi and not from TooTallNate.
I will merge #193 within the next few days though.

2013/8/2 Erhannis notifications@github.com

Yep. I tested it merged with the TooTallNate repo, as well as on its own,
but either way it still overflowed.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/190#issuecomment-21975906
.

[Erhannis]

No, I did; that stack trace at the top of the page was from before the fixes. Hang on, I'll get another stack trace.

[Erhannis]

Ugh, that was a pain. The error kept getting the bottom cut off, and...nevermind; anyway, here's a copy of the stack trace as given by Eclipse at a break on StackOverflowError:

Thread [<11> Thread-24](Suspended %28exception StackOverflowError%29)

ConnectionStateTLS(ConnectionState).getMinFragmentSize() line: 69
SSLRecordProtocol.getMinRecordSize() line: 134
SSLEngineImpl.unwrap(ByteBuffer, ByteBuffer[], int, int) line: 436
SSLEngineImpl(SSLEngine).unwrap(ByteBuffer, ByteBuffer) line: 385
SSLSocketChannel2.unwrap() line: 159
SSLSocketChannel2.readRemaining(ByteBuffer) line: 271
SSLSocketChannel2.read(ByteBuffer) line: 231
SSLSocketChannel2.read(ByteBuffer) line: 255
SSLSocketChannel2.read(ByteBuffer) line: 255
SSLSocketChannel2.read(ByteBuffer) line: 255
..................
SSLSocketChannel2.read(ByteBuffer) line: 255
SSLSocketChannel2.read(ByteBuffer) line: 255
SSLSocketChannel2.read(ByteBuffer) line: 255
SSLSocketChannel2.read(ByteBuffer) line: 255
SocketChannelIOHelper.read(ByteBuffer, WebSocketImpl, ByteChannel) line: 12
WebSocketManager$4(WebSocketClient).interruptableRun() line: 229
WebSocketManager$4(WebSocketClient).run() line: 188
Thread.run() line: 1019

[Erhannis]

So, I've stepped through a crash, and it looks like the following occurs:

1. When unwrap() is called, there's more data in inCrypt than there is space in inData.
2. sslEngine.unwrap( inCrypt, inData ) returns a status of BUFFER_OVERFLOW without unwrapping any of the data, so there's no data in inData.
3. transfereTo( inData, dst ) therefore transfers 0 bytes.
4. read( dst ) gets called again.

What OUGHT unwrap do when BUFFER_OVERFLOW occurs?

[Davidiusdadi]

Thank you those infos really help to isolate the problem!

So for some reason there is an overflow.

assert ( inData.position() == 0 );
inData.clear();

These lines should make sure that neither the buffers limit nor its position can cause the buffer to have to less bytes remaining.( I'd enable asserts anyway)

Since those can not be the problem i guess that the capacity must be the problem.

That would mean that createBuffer initialized indata with less that required bytes to hold the biggest application data buffer ( see http://docs.oracle.com/javase/1.5.0/docs/api/javax/net/ssl/SSLSession.html#getApplicationBufferSize() )

createBuffer() will be called in the constructor but it has to be called once more after the handshake. If for some reason that call did not happened that would be the bug.

One more possibility could be that the bug has to with rehandshaking. I never explicity tested rehandshaking. So it could be that createBuffer() is only buggy in combination with rehandshaking.

I will try to look at the issue as fast as possible but if you have the capacity to check in which way (position,limit,capaciy) indata is to small and if createBuffer is called appropriatly or if you experience rehandshakes that would really help out.

[Erhannis]

Here's the order of how things are happening:

1. createBuffer() is called once from the constructor.
   1.1. appBufferMax is set to 16384, affecting inData
   1.2. netBufferMax is set to 18437, affecting inCrypt and outData
2. processHandshake() is called once from the constructor.
3. processHandshake() is called 7 times from read().
4. processHandshake() is called once from write().
5. createBuffer() is NOT called again.
6. When the crash is happening, the state in unwrap() is:
   6.1. inCrypt:
   6.1.1. capacity = 18437
   6.1.2. position = 0
   6.1.3. limit = 18437
   6.2. inData:
   6.2.1. capacity = 16384
   6.2.2. position = 0
   6.2.3. limit = 16384
   6.3. sslEngine returns error code

Note that createBuffer() is only called once, as you said might be the problem.

[Erhannis]

Also, I've added some logging lines. The 7th time processHandshake() is called from read(), the engine handshake status starts as NEED_UNWRAP, then in the corresponding if block, after unwrap() the handshake state is NOT_HANDSHAKING. Is that supposed to work like that?

[Erhannis]

Oops, my mistake. That happened when...actually, one time it happened from read()->processHandshake(), and another time it happened from write()->processHandshake().
Also, that second time, there were a bunch of calls like write()->processHandshake() and THEN one call from read()->processHandshake(), so I suppose the order and count of the calls varies.

[Davidiusdadi]

I can reproduce that createBuffer is not called.