[tor-commits] [stem/master] Enforce VPaddingCell constructor specification of payload or size

[atagar at torproject.org]

commit c865744d4dded17ad46e637b8e116c41f251fbba
Author: Dave Rolek <dmr-x at riseup.net>
Date:   Wed May 30 20:45:15 2018 +0000

    Enforce VPaddingCell constructor specification of payload or size
    
    The behavior of a default range for VPADDING cells is not specified in
    the spec, and shouldn't be facilitated by stem.client.
    
    This change also removes a potentially network-visible use of insecure
    random generation.
    
    For stem.client randomness, we use secure generation via os.urandom()
    
    (Finally, this removes import of the random module, since it is no
    longer used.)
---
 stem/client/cell.py      | 6 ++++--
 test/unit/client/cell.py | 3 +++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/stem/client/cell.py b/stem/client/cell.py
index 3c923b35..d5e71e8a 100644
--- a/stem/client/cell.py
+++ b/stem/client/cell.py
@@ -40,7 +40,6 @@ Messages communicated over a Tor relay's ORPort.
 import datetime
 import inspect
 import os
-import random
 import sys
 
 import stem.util
@@ -588,7 +587,10 @@ class VPaddingCell(Cell):
 
   def __init__(self, size = None, payload = None):
     if payload is None:
-      payload = os.urandom(size) if size is not None else os.urandom(random.randint(128, 1024))
+      if size is not None:
+        payload = os.urandom(size)  # enforces size >= 0
+      else:
+        raise ValueError('VPaddingCell constructor must specify payload or size')
     elif size is not None and size != len(payload):
       raise ValueError('VPaddingCell constructor specified both a size of %i bytes and payload of %i bytes' % (size, len(payload)))
 
diff --git a/test/unit/client/cell.py b/test/unit/client/cell.py
index 93452195..785d48d2 100644
--- a/test/unit/client/cell.py
+++ b/test/unit/client/cell.py
@@ -5,6 +5,7 @@ Unit tests for the stem.client.cell.
 import datetime
 import hashlib
 import os
+import re
 import unittest
 
 from stem.client.datatype import ZERO, CertType, CloseReason, Address, Certificate
@@ -221,6 +222,8 @@ class TestCell(unittest.TestCase):
 
     self.assertRaisesRegexp(ValueError, 'VPaddingCell constructor specified both a size of 5 bytes and payload of 1 bytes', VPaddingCell, 5, '\x02')
 
+    self.assertRaisesRegexp(ValueError, '^%s$' % re.escape('VPaddingCell constructor must specify payload or size'), VPaddingCell)
+
   def test_certs_cell(self):
     for cell_bytes, certs in CERTS_CELLS.items():
       self.assertEqual(cell_bytes, CertsCell(certs).pack(2))