[tor-commits] [stem/master] Unpacking RELAY cells now checks data is the specified length

atagar at torproject.org atagar at torproject.org
Sat Jun 23 23:59:48 UTC 2018


commit 3df9e60af7ea968ffd14ddcd4a0cfc18345df50c
Author: Dave Rolek <dmr-x at 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





More information about the tor-commits mailing list