commit 3df9e60af7ea968ffd14ddcd4a0cfc18345df50c Author: Dave Rolek dmr-x@riseup.net Date: Sun Jun 3 19:34:45 2018 +0000

Unpacking RELAY cells now checks data is the specified length

Realistically, since RELAY cells are fixed-width, this was only a problem for cells which advertise a data length beyond (currently) 498 bytes, which is the max size that the data portion can be.

However, instead of hardcoding that assumption in, the implementation checks that the specified length of data is indeed unpacked. This should be more flexible, if ever the format changes in the future. --- stem/client/cell.py | 3 +++ test/unit/client/cell.py | 13 +++++++++++++ 2 files changed, 16 insertions(+)

diff --git a/stem/client/cell.py b/stem/client/cell.py index f000347d..a7019f35 100644 --- a/stem/client/cell.py +++ b/stem/client/cell.py @@ -374,6 +374,9 @@ class RelayCell(CircuitCell): data_len, content = Size.SHORT.pop(content) data, unused = split(content, data_len)

+ if len(data) != data_len: + raise ValueError('%s cell said it had %i bytes of data, but only had %i' % (cls.NAME, data_len, len(data))) + return RelayCell(circ_id, command, data, digest, stream_id, recognized, unused)

def __hash__(self): diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py index 93468318..16d01293 100644 --- a/test/unit/client/cell.py +++ b/test/unit/client/cell.py @@ -198,6 +198,19 @@ class TestCell(unittest.TestCase): self.assertRaisesRegexp(ValueError, 'RELAY cell digest must be a hash, string, or int but was a list', RelayCell, 5, 'RELAY_BEGIN_DIR', '', [], 564346860) self.assertRaisesRegexp(ValueError, "Invalid enumeration 'NO_SUCH_COMMAND', options are RELAY_BEGIN, RELAY_DATA", RelayCell, 5, 'NO_SUCH_COMMAND', '', 5, 564346860)

+ mismatched_data_length_bytes = b''.join(( + b'\x00\x01', # circ ID + b'\x03', # command + b'\x02', # relay command + b'\x00\x00', # 'recognized' + b'\x00\x01', # stream ID + b'\x15:m\xe0', # digest + b'\xFF\xFF', # data len (65535, clearly invalid) + ZERO * 498, # data + )) + expected_message = 'RELAY cell said it had 65535 bytes of data, but only had 498' + self.assertRaisesRegexp(ValueError, '^%s$' % re.escape(expected_message), Cell.pop, mismatched_data_length_bytes, 2) + def test_destroy_cell(self): for cell_bytes, (circ_id, reason, reason_int, unused) in DESTROY_CELLS.items(): # Packed cells always pad with zeros, so if we're testing something with