From c5d76cdd2e2e99d4ac65b645b17cf8a43e4ccab4 Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Tue, 8 Sep 2015 15:44:09 +0530 Subject: Do not use pickle: Use json Change-Id: Iffdd56704330897fbde21f101c9b2ed03c2ae296 Signed-off-by: Prashanth Pai Reviewed-by: Thiago da Silva Tested-by: Thiago da Silva Reviewed-on: http://review.gluster.org/13221 --- test/functional/tests.py | 12 +++++++ test/unit/common/test_utils.py | 82 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/test/functional/tests.py b/test/functional/tests.py index ad87d7e..b8633b0 100644 --- a/test/functional/tests.py +++ b/test/functional/tests.py @@ -1125,6 +1125,9 @@ class TestFile(Base): self.assert_status(400) def testMetadataNumberLimit(self): + raise SkipTest("Bad test") + # TODO(ppai): Fix it in upstream swift first + # Refer to comments below number_limit = load_constraint('max_meta_count') size_limit = load_constraint('max_meta_overall_size') @@ -1137,10 +1140,13 @@ class TestFile(Base): metadata = {} while len(metadata.keys()) < i: key = Utils.create_ascii_name() + # The following line returns a valid utf8 byte sequence val = Utils.create_name() if len(key) > j: key = key[:j] + # This slicing done below can make the 'utf8' byte + # sequence invalid and hence it cannot be decoded. val = val[:j] size += len(key) + len(val) @@ -1154,6 +1160,9 @@ class TestFile(Base): self.assert_status(201) self.assert_(file_item.sync_metadata()) self.assert_status((201, 202)) + self.assert_(file_item.initialize()) + self.assert_status(200) + self.assertEqual(file_item.metadata, metadata) else: self.assertRaises(ResponseError, file_item.write) self.assert_status(400) @@ -1315,6 +1324,9 @@ class TestFile(Base): self.assert_(file_item.write()) self.assert_status(201) self.assert_(file_item.sync_metadata()) + self.assert_(file_item.initialize()) + self.assert_status(200) + self.assertEqual(file_item.metadata, metadata) else: self.assertRaises(ResponseError, file_item.write) self.assert_status(400) diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 1fea1fc..0b173be 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -16,6 +16,7 @@ """ Tests for common.utils """ import os +import json import unittest import errno import xattr @@ -23,10 +24,12 @@ import tempfile import hashlib import tarfile import shutil +import cPickle as pickle from collections import defaultdict from mock import patch, Mock from gluster.swift.common import utils, Glusterfs -from gluster.swift.common.utils import deserialize_metadata, serialize_metadata +from gluster.swift.common.utils import deserialize_metadata, \ + serialize_metadata, PICKLE_PROTOCOL from gluster.swift.common.exceptions import GlusterFileSystemOSError,\ GlusterFileSystemIOError from swift.common.exceptions import DiskFileNoSpace @@ -138,6 +141,32 @@ def _mock_os_fsync(fd): return +class TestSafeUnpickler(unittest.TestCase): + + class Exploit(object): + def __reduce__(self): + return (os.system, ('touch /tmp/pickle-exploit',)) + + def test_loads(self): + valid_md = {'key1': 'val1', 'key2': 'val2'} + for protocol in (0, 1, 2): + valid_dump = pickle.dumps(valid_md, protocol) + mal_dump = pickle.dumps(self.Exploit(), protocol) + # malicious dump is appended to valid dump + payload1 = valid_dump[:-1] + mal_dump + # malicious dump is prefixed to valid dump + payload2 = mal_dump[:-1] + valid_dump + # entire dump is malicious + payload3 = mal_dump + for payload in (payload1, payload2, payload3): + try: + utils.SafeUnpickler.loads(payload) + except pickle.UnpicklingError as err: + self.assertTrue('Potentially unsafe pickle' in err) + else: + self.fail("Expecting cPickle.UnpicklingError") + + class TestUtils(unittest.TestCase): """ Tests for common.utils """ @@ -321,6 +350,57 @@ class TestUtils(unittest.TestCase): assert _xattr_op_cnt['get'] == 1, "%r" % _xattr_op_cnt assert _xattr_op_cnt['set'] == 0, "%r" % _xattr_op_cnt + def test_deserialize_metadata_pickle(self): + orig__read_pickled_metadata = Glusterfs._read_pickled_metadata + orig_md = {'key1': 'value1', 'key2': 'value2'} + pickled_md = pickle.dumps(orig_md, PICKLE_PROTOCOL) + _m_pickle_loads = Mock(return_value={}) + try: + with patch('gluster.swift.common.utils.pickle.loads', + _m_pickle_loads): + # Conf option turned off + Glusterfs._read_pickled_metadata = False + # pickled + utils.deserialize_metadata(pickled_md) + self.assertFalse(_m_pickle_loads.called) + _m_pickle_loads.reset_mock() + # not pickled + utils.deserialize_metadata("not_pickle") + self.assertFalse(_m_pickle_loads.called) + _m_pickle_loads.reset_mock() + + # Conf option turned on + Glusterfs._read_pickled_metadata = True + # pickled + md = utils.deserialize_metadata(pickled_md) + self.assertTrue(_m_pickle_loads.called) + self.assertTrue(isinstance(md, dict)) + _m_pickle_loads.reset_mock() + # not pickled + utils.deserialize_metadata("not_pickle") + self.assertFalse(_m_pickle_loads.called) + _m_pickle_loads.reset_mock() + + # malformed pickle + _m_pickle_loads.side_effect = pickle.UnpicklingError + md = utils.deserialize_metadata("malformed_pickle") + self.assertTrue(isinstance(md, dict)) + finally: + Glusterfs._read_pickled_metadata = orig__read_pickled_metadata + + def test_deserialize_metadata_json(self): + orig_md = {'key1': 'value1', 'key2': 'value2'} + json_md = json.dumps(orig_md, separators=(',', ':')) + _m_json_loads = Mock(return_value={}) + with patch('gluster.swift.common.utils.json.loads', + _m_json_loads): + utils.deserialize_metadata("not_json") + self.assertFalse(_m_json_loads.called) + _m_json_loads.reset_mock() + utils.deserialize_metadata("{fake_valid_json}") + self.assertTrue(_m_json_loads.called) + _m_json_loads.reset_mock() + def test_add_timestamp_empty(self): orig = {} res = utils._add_timestamp(orig) -- cgit