Skip to content

Commit 87f4072

Browse files
authored
Merge commit from fork
Motivation: We need to handle the situation correctly in all cases when there is not enough data yet to unwrap the packet. Not doing so might cause undefined behaviour Modifications: - Correctly check for SslUtils.NOT_ENOUGH_DATA - Add assert Result: Correctly handle incomplete packets while unwrap
1 parent d1fbda6 commit 87f4072

2 files changed

Lines changed: 19 additions & 6 deletions

File tree

handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,6 +1202,8 @@ public final SSLEngineResult unwrap(
12021202
throw new NotSslRecordException("not an SSL/TLS record");
12031203
}
12041204

1205+
assert packetLength >= 0;
1206+
12051207
final int packetLengthDataOnly = packetLength - SSL_RECORD_HEADER_LENGTH;
12061208
if (packetLengthDataOnly > capacity) {
12071209
// Not enough space in the destination buffer so signal the caller that the buffer needs to be

handler/src/main/java/io/netty/handler/ssl/SslUtils.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,8 +314,12 @@ static SSLHandshakeException toSSLHandshakeException(Throwable e) {
314314
* the given {@link ByteBuf} is not encrypted at all.
315315
*/
316316
static int getEncryptedPacketLength(ByteBuf buffer, int offset, boolean probeSSLv2) {
317+
assert offset >= buffer.readerIndex();
318+
int remaining = buffer.writerIndex() - offset;
319+
if (remaining < SSL_RECORD_HEADER_LENGTH) {
320+
return NOT_ENOUGH_DATA;
321+
}
317322
int packetLength = 0;
318-
319323
// SSLv3 or TLS - Check ContentType
320324
boolean tls;
321325
switch (buffer.getUnsignedByte(offset)) {
@@ -346,7 +350,7 @@ static int getEncryptedPacketLength(ByteBuf buffer, int offset, boolean probeSSL
346350
tls = false;
347351
}
348352
} else if (version == DTLS_1_0 || version == DTLS_1_2 || version == DTLS_1_3) {
349-
if (buffer.readableBytes() < offset + DTLS_RECORD_HEADER_LENGTH) {
353+
if (remaining < DTLS_RECORD_HEADER_LENGTH) {
350354
return NOT_ENOUGH_DATA;
351355
}
352356
// length is the last 2 bytes in the 13 byte header.
@@ -367,7 +371,8 @@ static int getEncryptedPacketLength(ByteBuf buffer, int offset, boolean probeSSL
367371
packetLength = headerLength == 2 ?
368372
(shortBE(buffer, offset) & 0x7FFF) + 2 : (shortBE(buffer, offset) & 0x3FFF) + 3;
369373
if (packetLength <= headerLength) {
370-
return NOT_ENOUGH_DATA;
374+
// If there's no data then consider this package as not encrypted.
375+
return NOT_ENCRYPTED;
371376
}
372377
} else {
373378
return NOT_ENCRYPTED;
@@ -420,24 +425,29 @@ static int getEncryptedPacketLength(ByteBuffer[] buffers, int offset) {
420425
}
421426

422427
// We need to copy 5 bytes into a temporary buffer so we can parse out the packet length easily.
423-
ByteBuffer tmp = ByteBuffer.allocate(5);
428+
ByteBuffer tmp = ByteBuffer.allocate(SSL_RECORD_HEADER_LENGTH);
424429

425430
do {
426431
buffer = buffers[offset++].duplicate();
427432
if (buffer.remaining() > tmp.remaining()) {
428433
buffer.limit(buffer.position() + tmp.remaining());
429434
}
430435
tmp.put(buffer);
431-
} while (tmp.hasRemaining());
436+
} while (tmp.hasRemaining() && offset < buffers.length);
432437

433438
// Done, flip the buffer so we can read from it.
434439
tmp.flip();
435440
return getEncryptedPacketLength(tmp);
436441
}
437442

438443
private static int getEncryptedPacketLength(ByteBuffer buffer) {
444+
int remaining = buffer.remaining();
445+
if (remaining < SSL_RECORD_HEADER_LENGTH) {
446+
return NOT_ENOUGH_DATA;
447+
}
439448
int packetLength = 0;
440449
int pos = buffer.position();
450+
441451
// SSLv3 or TLS - Check ContentType
442452
boolean tls;
443453
switch (unsignedByte(buffer.get(pos))) {
@@ -478,7 +488,8 @@ private static int getEncryptedPacketLength(ByteBuffer buffer) {
478488
packetLength = headerLength == 2 ?
479489
(shortBE(buffer, pos) & 0x7FFF) + 2 : (shortBE(buffer, pos) & 0x3FFF) + 3;
480490
if (packetLength <= headerLength) {
481-
return NOT_ENOUGH_DATA;
491+
// If there's no data then consider this package as not encrypted.
492+
return NOT_ENCRYPTED;
482493
}
483494
} else {
484495
return NOT_ENCRYPTED;

0 commit comments

Comments
 (0)