Perform signature verification on pickled data transferred over socke…

…ts (#251)

Perform signature verification on pickled data transferred over sockets

Before unpickling anything, ensure that it has a valid digital
signature using a randomly-generated shared key. In order for an attacker
to send or tamper with data on the same socket, they must know this key
to compute a valid signature. Fixes #105, CVE-2015-3539.

rope/base/oi/doa.py

@@ -1,7 +1,10 @@
+import base64
+import hashlib
+import hmac
 try:
-    import pickle
-except ImportError:
     import cPickle as pickle
+except ImportError:
+    import pickle
 import marshal
 import os
 import socket
@@ -11,6 +14,28 @@
 import threading
 
 
+def _compat_compare_digest(a, b):
+    """Implementation of hmac.compare_digest for python < 2.7.7.
+
+    This function uses an approach designed to prevent timing analysis by
+    avoiding content-based short circuiting behaviour, making it appropriate
+    for cryptography.
+    """
+    if len(a) != len(b):
+        return False
+    # Computes the bitwise difference of all characters in the two strings
+    # before returning whether or not they are equal.
+    difference = 0
+    for (a_char, b_char) in zip(a, b):
+        difference |= ord(a_char) ^ ord(b_char)
+    return difference == 0
+
+try:
+    from hmac import compare_digest
+except ImportError:
+    compare_digest = _compat_compare_digest
+
+
 class PythonFileRunner(object):
     """A class for running python project files"""
 
@@ -114,24 +139,55 @@ class _SocketReceiver(_MessageReceiver):
     def __init__(self):
         self.server_socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
         self.data_port = 3037
+        self.key = os.urandom(32)
+
         while self.data_port < 4000:
             try:
-                self.server_socket.bind(('', self.data_port))
+                self.server_socket.bind(('localhost', self.data_port))
                 break
             except socket.error:
                 self.data_port += 1
         self.server_socket.listen(1)
 
     def get_send_info(self):
-        return str(self.data_port)
+        return '%d:%s' % (self.data_port,
+                          base64.b64encode(self.key).decode('utf-8'))
 
     def receive_data(self):
         conn, addr = self.server_socket.accept()
         self.server_socket.close()
         my_file = conn.makefile('rb')
         while True:
+            # Received messages must meet the following criteria:
+            # 1. Must be contained on a single line.
+            # 2. Must be prefixed with a base64 encoded sha256 message digest 
+            #    of the base64 encoded pickle data.
+            # 3. Message digest must be computed using the correct key.
+            #
+            # Any messages received that do not meet these criteria will never
+            # be unpickled and will be dropped silently.
             try:
-                yield pickle.load(my_file)
+                buf = my_file.readline()
+                if len(buf) == 0:
+                    break
+
+                try:
+                    digest_end = buf.index(b':')
+                    buf_digest = base64.b64decode(buf[:digest_end])
+                    buf_data = buf[digest_end + 1:-1]
+                    decoded_buf_data = base64.b64decode(buf_data)
+                except:
+                    # Corrupted data; the payload cannot be trusted and just has
+                    # to be dropped. See CVE-2014-3539.
+                    continue
+
+                digest = hmac.new(self.key, buf_data, hashlib.sha256).digest()
+                if not compare_digest(buf_digest, digest):
+                    # Signature mismatch; the payload cannot be trusted and just
+                    # has to be dropped. See CVE-2014-3539.
+                    continue
+
+                yield pickle.loads(decoded_buf_data)
             except EOFError:
                 break
         my_file.close()

rope/base/oi/runmod.py

@@ -3,14 +3,17 @@ def __rope_start_everything():
     import sys
     import socket
     try:
-        import pickle
-    except ImportError:
         import cPickle as pickle
+    except ImportError:
+        import pickle
     import marshal
     import inspect
     import types
     import threading
     import rope.base.utils.pycompat as pycompat
+    import base64
+    import hashlib
+    import hmac
 
     class _MessageSender(object):
 
@@ -19,15 +22,19 @@ def send_data(self, data):
 
     class _SocketSender(_MessageSender):
 
-        def __init__(self, port):
+        def __init__(self, port, key):
             s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
             s.connect(('127.0.0.1', port))
             self.my_file = s.makefile('wb')
+            self.key = base64.b64decode(key)
 
         def send_data(self, data):
             if not self.my_file.closed:
-                pickle.dump(data, self.my_file)
-
+                pickled_data = base64.b64encode(
+                    pickle.dumps(data, pickle.HIGHEST_PROTOCOL))
+                dgst = hmac.new(self.key, pickled_data, hashlib.sha256).digest()
+                self.my_file.write(base64.b64encode(dgst) + b':' +
+                                   pickled_data + b'\n')
         def close(self):
             self.my_file.close()
 
@@ -58,8 +65,9 @@ class _FunctionCallDataSender(object):
 
         def __init__(self, send_info, project_root):
             self.project_root = project_root
-            if send_info.isdigit():
-                self.sender = _SocketSender(int(send_info))
+            if send_info[0].isdigit():
+                port, key = send_info.split(':', 1)
+                self.sender = _SocketSender(int(port), key)
             else:
                 self.sender = _FileSender(send_info)
 

rope/base/project.py

@@ -11,9 +11,9 @@
 from rope.base.resources import File, Folder, _ResourceMatcher
 
 try:
-    import pickle
-except ImportError:
     import cPickle as pickle
+except ImportError:
+    import pickle
 
 
 class _Project(object):

ropetest/__init__.py

@@ -6,6 +6,7 @@
 
 import ropetest.projecttest
 import ropetest.codeanalyzetest
+import ropetest.doatest
 import ropetest.type_hinting_test
 import ropetest.pycoretest
 import ropetest.pyscopestest
@@ -25,6 +26,7 @@ def suite():
     result = unittest.TestSuite()
     result.addTests(ropetest.projecttest.suite())
     result.addTests(ropetest.codeanalyzetest.suite())
+    result.addTests(ropetest.doatest.suite())
     result.addTests(ropetest.type_hinting_test.suite())
     result.addTests(ropetest.pycoretest.suite())
     result.addTests(ropetest.pyscopestest.suite())

ropetest/doatest.py

@@ -0,0 +1,91 @@
+import base64
+import hashlib
+import hmac
+import multiprocessing
+try:
+    import cPickle as pickle
+except ImportError:
+    import pickle
+import socket
+try:
+    import unittest2 as unittest
+except ImportError:
+    import unittest
+
+
+from rope.base.oi import doa
+
+
+class DOATest(unittest.TestCase):
+
+    def try_CVE_2014_3539_exploit(self, receiver, payload):
+        # Simulated attacker writing to the socket
+        def attacker(data_port):
+            s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+            s.connect(('127.0.0.1', data_port))
+            s_file = s.makefile('wb')
+            s_file.write(payload)
+            s.close()
+
+        # Assume the attacker guesses the port correctly; 3037 is used by
+        # default if it is available.
+        attacker_proc = multiprocessing.Process(target=attacker,
+                                                args=(receiver.data_port,))
+
+        attacker_proc.start()
+        received_objs = list(receiver.receive_data())
+        attacker_proc.join()
+        return received_objs
+
+    def test_CVE_2014_3539_no_encoding(self):
+        # Attacker sends pickled data to the receiver socket.
+        receiver = doa._SocketReceiver()
+
+        payload = pickle.dumps('def foo():\n    return 123\n')
+        received_objs = self.try_CVE_2014_3539_exploit(receiver, payload)
+
+        # Make sure the exploit did not run
+        self.assertEqual(0, len(received_objs))
+
+    def test_CVE_2014_3539_signature_mismatch(self):
+        # Attacker sends well-formed data with an incorrect signature.
+        receiver = doa._SocketReceiver()
+
+        pickled_data = pickle.dumps('def foo():\n    return 123\n',
+                                    pickle.HIGHEST_PROTOCOL)
+        digest = hmac.new(b'invalid-key', pickled_data, hashlib.sha256).digest()
+        payload = (base64.b64encode(digest) + b':' +
+                   base64.b64encode(pickled_data) + b'\n')
+        received_objs = self.try_CVE_2014_3539_exploit(receiver, payload)
+
+        # Make sure the exploit did not run
+        self.assertEqual(0, len(received_objs))
+
+    def test_CVE_2014_3539_sanity(self):
+        # Tests that sending valid, signed data on the socket does work.
+        receiver = doa._SocketReceiver()
+
+        pickled_data = base64.b64encode(
+            pickle.dumps('def foo():\n    return 123\n',
+                         pickle.HIGHEST_PROTOCOL))
+        digest = hmac.new(receiver.key, pickled_data, hashlib.sha256).digest()
+        payload = (base64.b64encode(digest) + b':' + pickled_data + b'\n')
+        received_objs = self.try_CVE_2014_3539_exploit(receiver, payload)
+
+        # Make sure the exploit did not run
+        self.assertEqual(1, len(received_objs))
+
+    def test_compare_digest_compat(self):
+        self.assertTrue(doa._compat_compare_digest('', ''))
+        self.assertTrue(doa._compat_compare_digest('abc', 'abc'))
+        self.assertFalse(doa._compat_compare_digest('abc', 'abd'))
+        self.assertFalse(doa._compat_compare_digest('abc', 'abcd'))
+
+
+def suite():
+    result = unittest.TestSuite()
+    result.addTests(unittest.makeSuite(DOATest))
+    return result
+
+if __name__ == '__main__':
+    unittest.main()