Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix possible IOOBE when calling ReferenceCountedSslEngine.unwrap(...)… #6236

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -422,12 +422,14 @@ private int writeEncryptedData(final ByteBuffer src, int len) {
final ByteBuf buf = alloc.directBuffer(len);
try {
final long addr = memoryAddress(buf);

// Set the limit depending on the length of the allocated buffer and restore the original
// after we copied bytes.
int limit = src.limit();
int newLimit = pos + len;
if (newLimit != src.remaining()) {
buf.setBytes(0, (ByteBuffer) src.duplicate().position(pos).limit(newLimit));
} else {
buf.setBytes(0, src);
}
src.limit(newLimit);
buf.setBytes(0, src);
src.limit(limit);

netWrote = SSL.writeToBIO(networkBIO, addr, len);
if (netWrote >= 0) {
Expand Down
75 changes: 75 additions & 0 deletions handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
Expand Up @@ -1323,4 +1323,79 @@ public void testCloseNotifySequence() throws Exception {
cleanupServerSslEngine(server);
}
}

@Test
public void testMultipleRecordsInOneBufferWithNonZeroPosition() throws Exception {
SelfSignedCertificate cert = new SelfSignedCertificate();

clientSslCtx = SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslProvider(sslClientProvider())
.build();
SSLEngine client = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);

serverSslCtx = SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslProvider(sslServerProvider())
.build();
SSLEngine server = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);

try {
// Choose buffer size small enough that we can put multiple buffers into one buffer and pass it into the
// unwrap call without exceed MAX_ENCRYPTED_PACKET_LENGTH.
ByteBuffer plainClientOut = ByteBuffer.allocate(1024);
ByteBuffer plainServerOut = ByteBuffer.allocate(server.getSession().getApplicationBufferSize());

ByteBuffer encClientToServer = ByteBuffer.allocate(client.getSession().getPacketBufferSize());

int positionOffset = 1;
// We need to be able to hold 2 records + positionOffset
ByteBuffer combinedEncClientToServer = ByteBuffer.allocate(
encClientToServer.capacity() * 2 + positionOffset);
combinedEncClientToServer.position(positionOffset);

handshake(client, server);

plainClientOut.limit(plainClientOut.capacity());
SSLEngineResult result = client.wrap(plainClientOut, encClientToServer);
assertEquals(plainClientOut.capacity(), result.bytesConsumed());
assertTrue(result.bytesProduced() > 0);

encClientToServer.flip();

// Copy the first record into the combined buffer
combinedEncClientToServer.put(encClientToServer);

plainClientOut.clear();
encClientToServer.clear();

result = client.wrap(plainClientOut, encClientToServer);
assertEquals(plainClientOut.capacity(), result.bytesConsumed());
assertTrue(result.bytesProduced() > 0);

encClientToServer.flip();

int encClientToServerLen = encClientToServer.remaining();

// Copy the first record into the combined buffer
combinedEncClientToServer.put(encClientToServer);

encClientToServer.clear();

combinedEncClientToServer.flip();
combinedEncClientToServer.position(positionOffset);

// Ensure we have the first record and a tiny amount of the second record in the buffer
combinedEncClientToServer.limit(
combinedEncClientToServer.limit() - (encClientToServerLen - positionOffset));
result = server.unwrap(combinedEncClientToServer, plainServerOut);
assertEquals(encClientToServerLen, result.bytesConsumed());
assertTrue(result.bytesProduced() > 0);
} finally {
cert.delete();
cleanupClientSslEngine(client);
cleanupServerSslEngine(server);
}
}
}