Strengthened frame body length checks.

This commit is contained in:
Simone Bordet 2014-06-06 12:09:11 +02:00
parent 2a485be6c1
commit 6481bb926d
14 changed files with 127 additions and 39 deletions

View File

@ -44,6 +44,11 @@ public class DataBodyParser extends BodyParser
@Override
protected boolean emptyBody()
{
if (isPaddingHigh() || isPaddingLow())
{
notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_data_frame");
return false;
}
return onData(BufferUtil.EMPTY_BUFFER, false);
}
@ -75,6 +80,10 @@ public class DataBodyParser extends BodyParser
{
paddingLength = (buffer.get() & 0xFF) << 8;
length -= 1;
if (length < 1 + 256)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_data_frame_padding");
}
state = State.PADDING_LOW;
break;
}
@ -82,6 +91,10 @@ public class DataBodyParser extends BodyParser
{
paddingLength += buffer.get() & 0xFF;
length -= 1;
if (length < paddingLength)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_data_frame_padding");
}
length -= paddingLength;
state = State.DATA;
break;

View File

@ -24,8 +24,9 @@ import org.eclipse.jetty.http2.frames.GoAwayFrame;
public class GoAwayBodyParser extends BodyParser
{
private State state = State.LAST_STREAM_ID;
private State state = State.PREPARE;
private int cursor;
private int length;
private int lastStreamId;
private int error;
private byte[] payload;
@ -37,8 +38,9 @@ public class GoAwayBodyParser extends BodyParser
private void reset()
{
state = State.LAST_STREAM_ID;
state = State.PREPARE;
cursor = 0;
length = 0;
lastStreamId = 0;
error = 0;
payload = null;
@ -51,6 +53,12 @@ public class GoAwayBodyParser extends BodyParser
{
switch (state)
{
case PREPARE:
{
state = State.LAST_STREAM_ID;
length = getBodyLength();
break;
}
case LAST_STREAM_ID:
{
if (buffer.remaining() >= 4)
@ -58,6 +66,11 @@ public class GoAwayBodyParser extends BodyParser
lastStreamId = buffer.getInt();
lastStreamId &= 0x7F_FF_FF_FF;
state = State.ERROR;
length -= 4;
if (length <= 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_go_away_frame");
}
}
else
{
@ -71,10 +84,19 @@ public class GoAwayBodyParser extends BodyParser
int currByte = buffer.get() & 0xFF;
--cursor;
lastStreamId += currByte << (8 * cursor);
--length;
if (cursor > 0 && length <= 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_go_away_frame");
}
if (cursor == 0)
{
lastStreamId &= 0x7F_FF_FF_FF;
state = State.ERROR;
if (length == 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_go_away_frame");
}
}
break;
}
@ -84,8 +106,12 @@ public class GoAwayBodyParser extends BodyParser
{
error = buffer.getInt();
state = State.PAYLOAD;
int payloadLength = getBodyLength() - 4 - 4;
if (payloadLength == 0)
length -= 4;
if (length < 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_go_away_frame");
}
if (length == 0)
{
return onGoAway(lastStreamId, error, null);
}
@ -102,11 +128,15 @@ public class GoAwayBodyParser extends BodyParser
int currByte = buffer.get() & 0xFF;
--cursor;
error += currByte << (8 * cursor);
--length;
if (cursor > 0 && length <= 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_go_away_frame");
}
if (cursor == 0)
{
state = State.PAYLOAD;
int payloadLength = getBodyLength() - 4 - 4;
if (payloadLength == 0)
if (length == 0)
{
return onGoAway(lastStreamId, error, null);
}
@ -115,9 +145,8 @@ public class GoAwayBodyParser extends BodyParser
}
case PAYLOAD:
{
int payloadLength = getBodyLength() - 4 - 4;
payload = new byte[payloadLength];
if (buffer.remaining() >= payloadLength)
payload = new byte[length];
if (buffer.remaining() >= length)
{
buffer.get(payload);
return onGoAway(lastStreamId, error, payload);
@ -125,7 +154,7 @@ public class GoAwayBodyParser extends BodyParser
else
{
state = State.PAYLOAD_BYTES;
cursor = payloadLength;
cursor = length;
}
break;
}
@ -157,6 +186,6 @@ public class GoAwayBodyParser extends BodyParser
private enum State
{
LAST_STREAM_ID, LAST_STREAM_ID_BYTES, ERROR, ERROR_BYTES, PAYLOAD, PAYLOAD_BYTES
PREPARE, LAST_STREAM_ID, LAST_STREAM_ID_BYTES, ERROR, ERROR_BYTES, PAYLOAD, PAYLOAD_BYTES
}
}

View File

@ -24,7 +24,7 @@ import org.eclipse.jetty.http2.frames.PingFrame;
public class PingBodyParser extends BodyParser
{
private State state = State.PAYLOAD;
private State state = State.PREPARE;
private int cursor;
private byte[] payload;
@ -35,7 +35,7 @@ public class PingBodyParser extends BodyParser
private void reset()
{
state = State.PAYLOAD;
state = State.PREPARE;
cursor = 0;
payload = null;
}
@ -47,6 +47,16 @@ public class PingBodyParser extends BodyParser
{
switch (state)
{
case PREPARE:
{
int length = getBodyLength();
if (length != 8)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_ping_frame");
}
state = State.PAYLOAD;
break;
}
case PAYLOAD:
{
payload = new byte[8];
@ -90,6 +100,6 @@ public class PingBodyParser extends BodyParser
private enum State
{
PAYLOAD, PAYLOAD_BYTES
PREPARE, PAYLOAD, PAYLOAD_BYTES
}
}

View File

@ -24,7 +24,7 @@ import org.eclipse.jetty.http2.frames.PriorityFrame;
public class PriorityBodyParser extends BodyParser
{
private State state = State.EXCLUSIVE;
private State state = State.PREPARE;
private int cursor;
private boolean exclusive;
private int streamId;
@ -36,7 +36,7 @@ public class PriorityBodyParser extends BodyParser
private void reset()
{
state = State.EXCLUSIVE;
state = State.PREPARE;
cursor = 0;
exclusive = false;
streamId = 0;
@ -49,6 +49,16 @@ public class PriorityBodyParser extends BodyParser
{
switch (state)
{
case PREPARE:
{
int length = getBodyLength();
if (length != 5)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_priority_frame");
}
state = State.EXCLUSIVE;
break;
}
case EXCLUSIVE:
{
// We must only peek the first byte and not advance the buffer
@ -108,6 +118,6 @@ public class PriorityBodyParser extends BodyParser
private enum State
{
EXCLUSIVE, STREAM_ID, STREAM_ID_BYTES, WEIGHT
PREPARE, EXCLUSIVE, STREAM_ID, STREAM_ID_BYTES, WEIGHT
}
}

View File

@ -24,7 +24,7 @@ import org.eclipse.jetty.http2.frames.ResetFrame;
public class ResetBodyParser extends BodyParser
{
private State state = State.ERROR;
private State state = State.PREPARE;
private int cursor;
private int error;
@ -35,7 +35,7 @@ public class ResetBodyParser extends BodyParser
private void reset()
{
state = State.ERROR;
state = State.PREPARE;
cursor = 0;
error = 0;
}
@ -47,6 +47,16 @@ public class ResetBodyParser extends BodyParser
{
switch (state)
{
case PREPARE:
{
int length = getBodyLength();
if (length != 4)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_rst_stream_frame");
}
state = State.ERROR;
break;
}
case ERROR:
{
if (buffer.remaining() >= 4)
@ -89,6 +99,6 @@ public class ResetBodyParser extends BodyParser
private enum State
{
ERROR, ERROR_BYTES
PREPARE, ERROR, ERROR_BYTES
}
}

View File

@ -66,10 +66,6 @@ public class SettingsBodyParser extends BodyParser
length = getBodyLength();
settings = new HashMap<>();
state = State.SETTING_ID;
if (length == 0)
{
return onSettings(settings);
}
break;
}
case SETTING_ID:
@ -77,9 +73,9 @@ public class SettingsBodyParser extends BodyParser
settingId = buffer.get() & 0xFF;
state = State.SETTING_VALUE;
--length;
if (length == 0)
if (length <= 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_settings");
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_settings_frame");
}
break;
}
@ -110,9 +106,9 @@ public class SettingsBodyParser extends BodyParser
--cursor;
settingValue += currByte << (8 * cursor);
--length;
if (cursor > 0 && length == 0)
if (cursor > 0 && length <= 0)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_settings");
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_settings_frame");
}
if (cursor == 0)
{

View File

@ -24,7 +24,7 @@ import org.eclipse.jetty.http2.frames.WindowUpdateFrame;
public class WindowUpdateBodyParser extends BodyParser
{
private State state = State.WINDOW_DELTA;
private State state = State.PREPARE;
private int cursor;
private int windowDelta;
@ -35,7 +35,7 @@ public class WindowUpdateBodyParser extends BodyParser
private void reset()
{
state = State.WINDOW_DELTA;
state = State.PREPARE;
cursor = 0;
windowDelta = 0;
}
@ -47,6 +47,16 @@ public class WindowUpdateBodyParser extends BodyParser
{
switch (state)
{
case PREPARE:
{
int length = getBodyLength();
if (length != 4)
{
return notifyConnectionFailure(ErrorCode.PROTOCOL_ERROR, "invalid_window_update_frame");
}
state = State.WINDOW_DELTA;
break;
}
case WINDOW_DELTA:
{
if (buffer.remaining() >= 4)
@ -91,6 +101,6 @@ public class WindowUpdateBodyParser extends BodyParser
private enum State
{
WINDOW_DELTA, WINDOW_DELTA_BYTES
PREPARE, WINDOW_DELTA, WINDOW_DELTA_BYTES
}
}

View File

@ -47,13 +47,19 @@ public class DataGenerateParseTest
@Test
public void testGenerateParseNoContentNoPadding()
{
ByteBuffer content = BufferUtil.EMPTY_BUFFER;
List<DataFrame> frames = testGenerateParse(0, content);
Assert.assertEquals(1, frames.size());
DataFrame frame = frames.get(0);
Assert.assertTrue(frame.getStreamId() != 0);
Assert.assertTrue(frame.isEnd());
Assert.assertEquals(content, frame.getData());
testGenerateParseContent(0, BufferUtil.EMPTY_BUFFER);
}
@Test
public void testGenerateParseNoContentSmallPadding()
{
testGenerateParseContent(128, BufferUtil.EMPTY_BUFFER);
}
@Test
public void testGenerateParseNoContentLargePadding()
{
testGenerateParseContent(1024, BufferUtil.EMPTY_BUFFER);
}
@Test
@ -76,7 +82,11 @@ public class DataGenerateParseTest
private void testGenerateParseSmallContent(int paddingLength)
{
ByteBuffer content = ByteBuffer.wrap(smallContent);
testGenerateParseContent(paddingLength, ByteBuffer.wrap(smallContent));
}
private void testGenerateParseContent(int paddingLength, ByteBuffer content)
{
List<DataFrame> frames = testGenerateParse(paddingLength, content);
Assert.assertEquals(1, frames.size());
DataFrame frame = frames.get(0);