Reject padded frames with invalid padding lengths

Lukasa · Lukasa · commit 6d63eae73170 · 2016-01-07T17:05:58.000Z
diff --git a/hyperframe/exceptions.py b/hyperframe/exceptions.py
@@ -23,3 +23,10 @@ def __str__(self):
             "UnknownFrameError: Unknown frame type 0x%X received, "
             "length %d bytes" % (self.frame_type, self.length)
         )
+
+
+class InvalidPaddingError(ValueError):
+    """
+    A frame with invalid padding was received.
+    """
+    pass
diff --git a/hyperframe/frame.py b/hyperframe/frame.py
@@ -10,7 +10,7 @@
 import collections
 import struct
 
-from .exceptions import UnknownFrameError
+from .exceptions import UnknownFrameError, InvalidPaddingError
 from .flags import Flag, Flags
 
 # The maximum initial length of a frame. Some frames have shorter maximum lengths.
@@ -232,6 +232,9 @@ def parse_body(self, data):
         self.data = data[padding_data_length:len(data)-self.total_padding].tobytes()
         self.body_len = len(data)
 
+        if self.total_padding >= self.body_len:
+            raise InvalidPaddingError("Padding is too long.")
+
     @property
     def flow_controlled_length(self):
         """
@@ -393,6 +396,9 @@ def parse_body(self, data):
         self.data = data[padding_data_length + 4:].tobytes()
         self.body_len = len(data)
 
+        if self.total_padding >= self.body_len:
+            raise InvalidPaddingError("Padding is too long.")
+
 
 class PingFrame(Frame):
     """
@@ -564,6 +570,9 @@ def parse_body(self, data):
         self.body_len = len(data)
         self.data = data[priority_data_length:len(data)-self.total_padding].tobytes()
 
+        if self.total_padding >= self.body_len:
+            raise InvalidPaddingError("Padding is too long.")
+
 
 class ContinuationFrame(Frame):
     """
diff --git a/test/test_frames.py b/test/test_frames.py
@@ -4,7 +4,7 @@
     PushPromiseFrame, PingFrame, GoAwayFrame, WindowUpdateFrame, HeadersFrame,
     ContinuationFrame, AltSvcFrame, Origin, BlockedFrame,
 )
-from hyperframe.exceptions import UnknownFrameError
+from hyperframe.exceptions import UnknownFrameError, InvalidPaddingError
 import pytest
 
 
@@ -146,6 +146,14 @@ def test_body_length_behaves_correctly(self):
         data = f.serialize()
         assert f.body_len == 300
 
+    def test_data_frame_with_invalid_padding_fails_to_parse(self):
+        # This frame has a padding length of 6 bytes, but a total length of
+        # only 5.
+        data = b'\x00\x00\x05\x00\x0b\x00\x00\x00\x01\x06\x54\x65\x73\x74'
+
+        with pytest.raises(InvalidPaddingError):
+            decode_frame(data)
+
 
 class TestPriorityFrame(object):
     payload = b'\x00\x00\x05\x02\x00\x00\x00\x00\x01\x80\x00\x00\x04\x40'
@@ -308,6 +316,14 @@ def test_push_promise_frame_parses_properly(self):
         assert f.data == b'hello world'
         assert f.body_len == 15
 
+    def test_push_promise_frame_with_invalid_padding_fails_to_parse(self):
+        # This frame has a padding length of 6 bytes, but a total length of
+        # only 5.
+        data = b'\x00\x00\x05\x05\x08\x00\x00\x00\x01\x06\x54\x65\x73\x74'
+
+        with pytest.raises(InvalidPaddingError):
+            decode_frame(data)
+
 
 class TestPingFrame(object):
     def test_ping_frame_has_only_one_flag(self):
@@ -482,6 +498,14 @@ def test_headers_frame_with_priority_serializes_properly(self):
 
         assert f.serialize() == s
 
+    def test_headers_frame_with_invalid_padding_fails_to_parse(self):
+        # This frame has a padding length of 6 bytes, but a total length of
+        # only 5.
+        data = b'\x00\x00\x05\x01\x08\x00\x00\x00\x01\x06\x54\x65\x73\x74'
+
+        with pytest.raises(InvalidPaddingError):
+            decode_frame(data)
+
 
 class TestContinuationFrame(object):
     def test_continuation_frame_flags(self):
