Skip to content

Commit b2d5196

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. This changes the code to use the `json` package instead of the `marshal` package and extracts the data more carefully.
1 parent 9f3a2ee commit b2d5196

File tree

2 files changed

+53
-7
lines changed

2 files changed

+53
-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: 41 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

@@ -594,6 +595,46 @@ def test_save_bad_type(self):
594595
with self.assertRaises(TypeError):
595596
save_operator('ping', 'somewhere')
596597

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

598639
class GetFileDirTest(unittest.TestCase):
599640
def setUp(self):

0 commit comments

Comments
 (0)