Skip to content

Commit 3f5aeef

Browse files
committed
Fix deserialization warning
CodeQL security [scan report #567](https://github.com/quantumlib/OpenFermion/security/code-scanning/567) flagged a data loading operation in `src/openfermion/utils/operator_utils.py` as being usafe due because it uses a user-provided value. The warning is about lines 282-283, involving the code ```python with open(file_path, 'rb') as f: data = marshal.load(f) ``` > Deserializing untrusted data using any deserialization framework that allows the construction of arbitrary serializable objects is easily exploitable and in many cases allows an attacker to execute arbitrary code. Even before a deserialized object is returned to the caller of a deserialization method a lot of code may have been executed, including static initializers, constructors, and finalizers. Automatic deserialization of fields means that an attacker may craft a nested combination of objects on which the executed initialization code may have unforeseen effects, such as the execution of arbitrary code.
1 parent 9f3a2ee commit 3f5aeef

File tree

2 files changed

+60
-7
lines changed

2 files changed

+60
-7
lines changed

src/openfermion/utils/operator_utils.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@
1111
# limitations under the License.
1212
"""This module provides generic tools for classes in ops/"""
1313
from builtins import map, zip
14-
import marshal
14+
import json
1515
import os
16+
from ast import literal_eval
1617

1718
import numpy
1819
import sympy
@@ -279,10 +280,13 @@ def load_operator(file_name=None, data_directory=None, plain_text=False):
279280
else:
280281
raise TypeError('Operator of invalid type.')
281282
else:
282-
with open(file_path, 'rb') as f:
283-
data = marshal.load(f)
284-
operator_type = data[0]
285-
operator_terms = data[1]
283+
with open(file_path, 'r') as f:
284+
data = json.load(f)
285+
operator_type, serializable_terms = data
286+
operator_terms = {
287+
literal_eval(key): complex(value[0], value[1])
288+
for key, value in serializable_terms.items()
289+
}
286290

287291
if operator_type == 'FermionOperator':
288292
operator = FermionOperator()
@@ -356,5 +360,6 @@ def save_operator(
356360
f.write(operator_type + ":\n" + str(operator))
357361
else:
358362
tm = operator.terms
359-
with open(file_path, 'wb') as f:
360-
marshal.dump((operator_type, dict(zip(tm.keys(), map(complex, tm.values())))), f)
363+
serializable_terms = {str(key): (value.real, value.imag) for key, value in tm.items()}
364+
with open(file_path, 'w') as f:
365+
json.dump((operator_type, serializable_terms), f)

src/openfermion/utils/operator_utils_test.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
"""Tests for operator_utils."""
1313

1414
import os
15+
import json
1516

1617
import itertools
1718

@@ -473,13 +474,20 @@ def setUp(self):
473474
with open(os.path.join(DATA_DIRECTORY, self.bad_operator_filename), 'w') as fid:
474475
fid.write(bad_op)
475476

477+
self.bad_type_filename = 'bad_type_operator.data'
478+
with open(os.path.join(DATA_DIRECTORY, self.bad_type_filename), 'w') as f:
479+
json.dump(['InvalidOperatorType', {}], f)
480+
476481
def tearDown(self):
477482
file_path = os.path.join(DATA_DIRECTORY, self.file_name + '.data')
478483
if os.path.isfile(file_path):
479484
os.remove(file_path)
480485
file_path = os.path.join(DATA_DIRECTORY, self.bad_operator_filename)
481486
if os.path.isfile(file_path):
482487
os.remove(file_path)
488+
file_path = os.path.join(DATA_DIRECTORY, self.bad_type_filename)
489+
if os.path.isfile(file_path):
490+
os.remove(file_path)
483491

484492
def test_save_sympy_plaintext(self):
485493
operator = FermionOperator('1^', sympy.Symbol('x'))
@@ -594,6 +602,46 @@ def test_save_bad_type(self):
594602
with self.assertRaises(TypeError):
595603
save_operator('ping', 'somewhere')
596604

605+
def test_save_and_load_complex_json(self):
606+
fermion_op = FermionOperator('1^ 2', 1 + 2j)
607+
boson_op = BosonOperator('1^ 2', 1 + 2j)
608+
qubit_op = QubitOperator('X1 Y2', 1 + 2j)
609+
quad_op = QuadOperator('q1 p2', 1 + 2j)
610+
611+
save_operator(fermion_op, self.file_name)
612+
loaded_op = load_operator(self.file_name)
613+
self.assertEqual(fermion_op, loaded_op)
614+
615+
save_operator(boson_op, self.file_name, allow_overwrite=True)
616+
loaded_op = load_operator(self.file_name)
617+
self.assertEqual(boson_op, loaded_op)
618+
619+
save_operator(qubit_op, self.file_name, allow_overwrite=True)
620+
loaded_op = load_operator(self.file_name)
621+
self.assertEqual(qubit_op, loaded_op)
622+
623+
save_operator(quad_op, self.file_name, allow_overwrite=True)
624+
loaded_op = load_operator(self.file_name)
625+
self.assertEqual(quad_op, loaded_op)
626+
627+
def test_saved_json_content(self):
628+
import json
629+
630+
qubit_op = QubitOperator('X1 Y2', 1 + 2j)
631+
save_operator(qubit_op, self.file_name)
632+
633+
file_path = get_file_path(self.file_name, None)
634+
with open(file_path, 'r') as f:
635+
data = json.load(f)
636+
637+
self.assertEqual(len(data), 2)
638+
self.assertEqual(data[0], 'QubitOperator')
639+
640+
# The key is stringified tuple
641+
# The value is a list [real, imag]
642+
expected_terms = {"((1, 'X'), (2, 'Y'))": [1.0, 2.0]}
643+
self.assertEqual(data[1], expected_terms)
644+
597645

598646
class GetFileDirTest(unittest.TestCase):
599647
def setUp(self):

0 commit comments

Comments
 (0)