From 8016fb6d0f66a66aec04095ffc0fb60c0ec64cf9 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 24 Jun 2014 10:49:52 +0200 Subject: [PATCH] Strengthened the checks to avoid to exceed the max frame length. --- .../jetty/http2/generator/GoAwayGenerator.java | 11 ++++++++++- .../jetty/http2/generator/PushPromiseGenerator.java | 9 ++++++--- .../jetty/http2/generator/SettingsGenerator.java | 8 +++++++- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java index 121059875f0..e88aa611659 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/GoAwayGenerator.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.http2.generator; import java.nio.ByteBuffer; +import java.util.Arrays; import org.eclipse.jetty.http2.frames.Flag; import org.eclipse.jetty.http2.frames.Frame; @@ -46,7 +47,15 @@ public class GoAwayGenerator extends FrameGenerator if (lastStreamId < 0) throw new IllegalArgumentException("Invalid last stream id: " + lastStreamId); - int length = 4 + 4 + (payload != null ? payload.length : 0); + // The last streamId + the error code. + int fixedLength = 4 + 4; + + // Make sure we don't exceed the frame max length. + int maxPayloadLength = Frame.MAX_LENGTH - fixedLength; + if (payload != null && payload.length > maxPayloadLength) + payload = Arrays.copyOfRange(payload, 0, maxPayloadLength); + + int length = fixedLength + (payload != null ? payload.length : 0); ByteBuffer header = generateHeader(lease, FrameType.GO_AWAY, length, Flag.NONE, 0); header.putInt(lastStreamId); diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java index 4c7bcb60bb9..db074e682e9 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/PushPromiseGenerator.java @@ -20,12 +20,12 @@ package org.eclipse.jetty.http2.generator; import java.nio.ByteBuffer; +import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.http2.frames.Flag; import org.eclipse.jetty.http2.frames.Frame; import org.eclipse.jetty.http2.frames.FrameType; import org.eclipse.jetty.http2.frames.PushPromiseFrame; import org.eclipse.jetty.http2.hpack.HpackEncoder; -import org.eclipse.jetty.http.MetaData; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.util.BufferUtil; @@ -55,12 +55,15 @@ public class PushPromiseGenerator extends FrameGenerator encoder.encode(metaData, lease); + // The promised streamId. + int fixedLength = 4; + long length = lease.getTotalLength(); - if (length > Frame.MAX_LENGTH) + if (length > Frame.MAX_LENGTH - fixedLength) throw new IllegalArgumentException("Invalid headers, too big"); // Space for the promised streamId. - length += 4; + length += fixedLength; int flags = Flag.END_HEADERS; diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/SettingsGenerator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/SettingsGenerator.java index 43971341750..87e5fbe95c7 100644 --- a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/SettingsGenerator.java +++ b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/SettingsGenerator.java @@ -44,7 +44,13 @@ public class SettingsGenerator extends FrameGenerator public void generateSettings(ByteBufferPool.Lease lease, Map settings, boolean reply) { - ByteBuffer header = generateHeader(lease, FrameType.SETTINGS, 5 * settings.size(), reply ? Flag.ACK : Flag.NONE, 0); + // One byte for the identifier, 4 bytes for the value. + int entryLength = 1 + 4; + int length = entryLength * settings.size(); + if (length > Frame.MAX_LENGTH) + throw new IllegalArgumentException("Invalid settings, too big"); + + ByteBuffer header = generateHeader(lease, FrameType.SETTINGS, length, reply ? Flag.ACK : Flag.NONE, 0); for (Map.Entry entry : settings.entrySet()) {