Skip to content

Commit

Permalink
BUG: load fails when using pickle without allow_pickle=True
Browse files Browse the repository at this point in the history
a partial mitigation of #12759.

see also https://nvd.nist.gov/vuln/detail/CVE-2019-6446
  • Loading branch information
ivanov authored and charris committed Apr 17, 2019
1 parent cc94f36 commit a4df7e5
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 16 deletions.
5 changes: 5 additions & 0 deletions doc/release/1.17.0-notes.rst
Expand Up @@ -77,6 +77,11 @@ non-functional. This code was removed. If needed, use
If the out argument to these functions is provided and has memory overlap with
the other arguments, it is now buffered to avoid order-dependent behavior.

Unpickling while loading requires explicit opt-in
-------------------------------------------------
The functions ``np.load``, and ``np.lib.format.read_array`` take an
`allow_pickle` keyword which now defaults to ``False`` in response to
`CVE-2019-6446 <https://nvd.nist.gov/vuln/detail/CVE-2019-6446>`_.

C API changes
=============
Expand Down
2 changes: 1 addition & 1 deletion numpy/core/tests/test_regression.py
Expand Up @@ -98,7 +98,7 @@ def test_char_dump(self):
f = BytesIO()
pickle.dump(ca, f, protocol=proto)
f.seek(0)
ca = np.load(f)
ca = np.load(f, allow_pickle=True)
f.close()

def test_noncontiguous_fill(self):
Expand Down
10 changes: 7 additions & 3 deletions numpy/lib/format.py
Expand Up @@ -149,7 +149,7 @@
Notes
-----
The ``.npy`` format, including motivation for creating it and a comparison of
alternatives, is described in the `"npy-format" NEP
alternatives, is described in the `"npy-format" NEP
<https://www.numpy.org/neps/nep-0001-npy-format.html>`_, however details have
evolved with time and this document is more current.
Expand Down Expand Up @@ -644,7 +644,7 @@ def write_array(fp, array, version=None, allow_pickle=True, pickle_kwargs=None):
fp.write(chunk.tobytes('C'))


def read_array(fp, allow_pickle=True, pickle_kwargs=None):
def read_array(fp, allow_pickle=False, pickle_kwargs=None):
"""
Read an array from an NPY file.
Expand All @@ -654,7 +654,11 @@ def read_array(fp, allow_pickle=True, pickle_kwargs=None):
If this is not a real file object, then this may take extra memory
and time.
allow_pickle : bool, optional
Whether to allow reading pickled data. Default: True
Whether to allow writing pickled data. Default: False
.. versionchanged:: 1.16.3
Made default False in response to CVE-2019-6446.
pickle_kwargs : dict
Additional keyword arguments to pass to pickle.load. These are only
useful when loading object arrays saved on Python 2 when using
Expand Down
17 changes: 12 additions & 5 deletions numpy/lib/npyio.py
Expand Up @@ -146,7 +146,11 @@ class NpzFile(Mapping):
An object on which attribute can be performed as an alternative
to getitem access on the `NpzFile` instance itself.
allow_pickle : bool, optional
Allow loading pickled data. Default: True
Allow loading pickled data. Default: False
.. versionchanged:: 1.16.3
Made default False in response to CVE-2019-6446.
pickle_kwargs : dict, optional
Additional keyword arguments to pass on to pickle.load.
These are only useful when loading object arrays saved on
Expand Down Expand Up @@ -182,7 +186,7 @@ class NpzFile(Mapping):
"""

def __init__(self, fid, own_fid=False, allow_pickle=True,
def __init__(self, fid, own_fid=False, allow_pickle=False,
pickle_kwargs=None):
# Import is postponed to here since zipfile depends on gzip, an
# optional component of the so-called standard library.
Expand Down Expand Up @@ -285,7 +289,7 @@ def iterkeys(self):


@set_module('numpy')
def load(file, mmap_mode=None, allow_pickle=True, fix_imports=True,
def load(file, mmap_mode=None, allow_pickle=False, fix_imports=True,
encoding='ASCII'):
"""
Load arrays or pickled objects from ``.npy``, ``.npz`` or pickled files.
Expand Down Expand Up @@ -313,8 +317,11 @@ def load(file, mmap_mode=None, allow_pickle=True, fix_imports=True,
Allow loading pickled object arrays stored in npy files. Reasons for
disallowing pickles include security, as loading pickled data can
execute arbitrary code. If pickles are disallowed, loading object
arrays will fail.
Default: True
arrays will fail. Default: False
.. versionchanged:: 1.16.3
Made default False in response to CVE-2019-6446.
fix_imports : bool, optional
Only useful when loading Python 2 generated pickled files on Python 3,
which includes npy/npz files containing object arrays. If `fix_imports`
Expand Down
15 changes: 9 additions & 6 deletions numpy/lib/tests/test_format.py
Expand Up @@ -426,7 +426,7 @@ def roundtrip(arr):
f = BytesIO()
format.write_array(f, arr)
f2 = BytesIO(f.getvalue())
arr2 = format.read_array(f2)
arr2 = format.read_array(f2, allow_pickle=True)
return arr2


Expand Down Expand Up @@ -576,7 +576,7 @@ def test_pickle_python2_python3():
path = os.path.join(data_dir, fname)

for encoding in ['bytes', 'latin1']:
data_f = np.load(path, encoding=encoding)
data_f = np.load(path, allow_pickle=True, encoding=encoding)
if fname.endswith('.npz'):
data = data_f['x']
data_f.close()
Expand All @@ -598,16 +598,19 @@ def test_pickle_python2_python3():
if sys.version_info[0] >= 3:
if fname.startswith('py2'):
if fname.endswith('.npz'):
data = np.load(path)
data = np.load(path, allow_pickle=True)
assert_raises(UnicodeError, data.__getitem__, 'x')
data.close()
data = np.load(path, fix_imports=False, encoding='latin1')
data = np.load(path, allow_pickle=True, fix_imports=False,
encoding='latin1')
assert_raises(ImportError, data.__getitem__, 'x')
data.close()
else:
assert_raises(UnicodeError, np.load, path)
assert_raises(UnicodeError, np.load, path,
allow_pickle=True)
assert_raises(ImportError, np.load, path,
encoding='latin1', fix_imports=False)
allow_pickle=True, fix_imports=False,
encoding='latin1')


def test_pickle_disallow():
Expand Down
2 changes: 1 addition & 1 deletion numpy/lib/tests/test_io.py
Expand Up @@ -87,7 +87,7 @@ def roundtrip(self, save_func, *args, **kwargs):
"""
save_kwds = kwargs.get('save_kwds', {})
load_kwds = kwargs.get('load_kwds', {})
load_kwds = kwargs.get('load_kwds', {"allow_pickle": True})
file_on_disk = kwargs.get('file_on_disk', False)

if file_on_disk:
Expand Down

0 comments on commit a4df7e5

Please sign in to comment.