Fix gzip decompression bomb vulnerability (CWE-409) - #112
Conversation
|
Thanks for the report and the patch. The chunked read is the right mechanism, but I can see users compressing >100MB of numpy arrays pretty commonly, so a hard 100MB default would break existing workflows: files written with I'd suggest reshaping it along these lines:
On the safety front, since you can import modules and instantiate classes with the decoder the operation is already "unsafe" but I think this knob is nice to have with a default of |
|
Thanks @claydugo for the thorough review — all four points addressed in 9ea84ed. The hard 100MB default is gone. 1. Module-level setting, default Added import json_tricks.utils
json_tricks.utils.DECOMPRESSION_LIMIT = 256 * 1024 * 1024 # opt in for untrusted input(Users set it on 2. ndarray
3. Tests use a tiny limit and tiny payloads New 4. Safety note Agreed — loading untrusted JSON with this decoder is already unsafe (class instantiation via |
|
Maybe getting too big of a diff here. Not big on AI's taste for testing. Can we just make a 1 good test in a preexisting file? Can I suggest patchdiff --git a/json_tricks/__init__.py b/json_tricks/__init__.py
index 09739f7..af85f1b 100644
--- a/json_tricks/__init__.py
+++ b/json_tricks/__init__.py
@@ -3,8 +3,7 @@ try:
from json import JSONDecodeError # imported for convenience
except ImportError:
""" Older versions of Python use ValueError, of which JSONDecodeError is a subclass; it's recommended to catch ValueError. """
-from .utils import hashodict, NoEnumException, NoNumpyException, NoPandasException, get_scalar_repr, encode_intenums_inplace, encode_scalars_inplace, \
- DecompressionBombError, DecompressionBombWarning
+from .utils import hashodict, NoEnumException, NoNumpyException, NoPandasException, get_scalar_repr, encode_intenums_inplace, encode_scalars_inplace
from .comment import strip_comment_line_with_symbol, strip_comments
from .encoders import TricksEncoder, json_date_time_encode, class_instance_encode, json_complex_encode, \
numeric_types_encode, ClassInstanceEncoder, json_set_encode, pandas_encode, nopandas_encode, \
diff --git a/json_tricks/decoders.py b/json_tricks/decoders.py
index 6bb480e..8cd75eb 100644
--- a/json_tricks/decoders.py
+++ b/json_tricks/decoders.py
@@ -7,8 +7,7 @@ from decimal import Decimal
from fractions import Fraction
from json_tricks import NoEnumException, NoPandasException, NoNumpyException
-from .utils import ClassInstanceHookBase, nested_index, str_type, gzip_decompress, filtered_wrapper, \
- check_decompression_size
+from .utils import ClassInstanceHookBase, nested_index, str_type, gzip_decompress, filtered_wrapper
class DuplicateJsonKeyException(Exception):
@@ -306,22 +305,20 @@ def _bin_str_to_ndarray(data, order, shape, np_type_name, data_endianness):
From base64 encoded, gzipped binary data to ndarray.
"""
from base64 import standard_b64decode
- from numpy import frombuffer, dtype, prod
+ from numpy import frombuffer, dtype
assert order in [None, 'C'], 'specifying different memory order is not (yet) supported ' \
'for binary numpy format (got order = {})'.format(order)
np_type = dtype(np_type_name)
if data.startswith('b64.gz:'):
data = standard_b64decode(data[7:])
- # The expected decompressed size is known exactly from shape and dtype, so we
- # can bound memory without any configurable limit and detect surplus data
- # (a gzip decompression bomb, CWE-409). The exact-cap decompress raises if
- # the stream produces more than ``expected_bytes``.
- expected_bytes = int(prod(shape) or 1) * np_type.itemsize
- # If a module-level limit is set, check the *claimed* size before decompressing.
- check_decompression_size(expected_bytes, 'Numpy array of shape {} and dtype {}'
- .format(shape, np_type_name))
- data = gzip_decompress(data, max_size=expected_bytes, exact=True)
+ # the encoder writes exactly size * itemsize bytes, so anything beyond that is corrupt or hostile
+ expected_bytes = np_type.itemsize
+ for dimension in shape:
+ expected_bytes *= dimension
+ if expected_bytes < 0:
+ raise ValueError('numpy array has invalid shape {}'.format(shape))
+ data = gzip_decompress(data, max_size=expected_bytes)
elif data.startswith('b64:'):
data = standard_b64decode(data[4:])
else:
@@ -356,6 +353,12 @@ def _lists_of_obj_to_ndarray(data, order, shape, dtype):
From nested list of objects (that aren't native numpy numbers) to ndarray.
"""
from numpy import empty, ndindex
+ # the declared shape must be backed by real data, or it alone would size the allocation
+ level = [data]
+ for size in shape:
+ if any(not isinstance(node, (list, tuple)) or len(node) != size for node in level):
+ raise ValueError('nested data does not match declared shape {}'.format(shape))
+ level = [item for node in level for item in node]
arr = empty(shape, dtype=dtype, order=order)
dec_data = data
for indx in ndindex(arr.shape):
diff --git a/json_tricks/utils.py b/json_tricks/utils.py
index 8ad5895..12b97ef 100644
--- a/json_tricks/utils.py
+++ b/json_tricks/utils.py
@@ -75,24 +75,6 @@ class NoPandasException(Exception):
""" Trying to use pandas features, but pandas cannot be found. """
-class DecompressionBombError(Exception):
- """ Raised when decompressed data exceeds the configured safety limit (CWE-409). """
-
-
-class DecompressionBombWarning(UserWarning):
- """ Issued when decompressed data exceeds the safety limit but not yet 2x. """
-
-
-# Module-level decompression bomb limit (CWE-409). ``None`` (default) disables
-# checking, preserving backward compatibility for trusted input. Set to an int
-# (bytes) to enable Pillow-style protection: a DecompressionBombWarning is issued
-# once the decompressed size exceeds the limit, and a DecompressionBombError is
-# raised once it exceeds twice the limit. The setting is consulted by both
-# ``loads()`` (auto-decompression of gzip-compressed JSON) and the numpy
-# ``b64.gz:`` ndarray hook, without threading a new parameter through the API.
-DECOMPRESSION_LIMIT = None
-
-
class NoEnumException(Exception):
""" Trying to use enum features, but enum cannot be found. """
@@ -217,79 +199,21 @@ def gzip_compress(data, compresslevel):
return buf.getvalue()
-def check_decompression_size(claimed_bytes, context=''):
- """
- Check a *claimed* decompressed size (known before decompressing) against the
- module-level ``DECOMPRESSION_LIMIT``, if set. Raises ``DecompressionBombError`` at
- 2x the limit and issues a ``DecompressionBombWarning`` at 1x. Used by the numpy
- ndarray path where ``shape`` and ``dtype`` give the exact expected byte count up
- front. Reads ``DECOMPRESSION_LIMIT`` at call time, so updating
- ``json_tricks.utils.DECOMPRESSION_LIMIT`` takes effect immediately.
-
- :param claimed_bytes: The exact byte count the caller expects to decompress.
- :param context: Optional human-readable description for the warning/error message.
- """
- limit = DECOMPRESSION_LIMIT
- if limit is None:
- return
- if claimed_bytes > 2 * limit:
- raise DecompressionBombError(
- '{} claims {} bytes, exceeding 2x the safety limit of {} bytes; '
- 'this looks like a decompression bomb (CWE-409). Set '
- 'json_tricks.utils.DECOMPRESSION_LIMIT to None to allow it.'.format(context, claimed_bytes, limit))
- if claimed_bytes > limit:
- warnings.warn(
- '{} claims {} bytes, exceeding the safety limit of {} bytes (CWE-409). '
- 'Set json_tricks.utils.DECOMPRESSION_LIMIT to None to silence.'.format(context, claimed_bytes, limit),
- DecompressionBombWarning)
-
-
-def gzip_decompress(data, max_size=None, exact=False):
+def gzip_decompress(data, max_size=None):
"""
Do gzip decompression, without the timestamp. Just like gzip.decompress, but that's py3.2+.
- Decompression bomb protection (CWE-409): when ``max_size`` is given (or, when
- ``None``, falls back to the module-level ``DECOMPRESSION_LIMIT``), the decompressed
- size is checked while reading. In the default (``exact=False``) Pillow-style mode, a
- ``DecompressionBombWarning`` is issued once the size exceeds the limit and a
- ``DecompressionBombError`` is raised once it exceeds twice the limit. In ``exact=True``
- mode, a ``DecompressionBombError`` is raised as soon as the size exceeds the limit
- (used when the exact expected byte count is known, e.g. numpy ndarray buffers). A
- limit of ``None`` disables checking entirely, preserving backward compatibility.
-
- :param max_size: Per-call override in bytes. If ``None``, ``DECOMPRESSION_LIMIT`` is used.
- :param exact: If ``True``, raise immediately on exceeding ``max_size`` (no 2x buffer).
+ :param max_size: If given, decompress at most this many bytes, and raise if the stream
+ holds more. Pass it when the size is known in advance, so that a corrupt or hostile
+ stream cannot expand without bound (CWE-409).
"""
- limit = max_size if max_size is not None else DECOMPRESSION_LIMIT
- warned = False
- result = bytearray()
- with gzip.GzipFile(fileobj=io.BytesIO(data)) as f:
- while True:
- chunk = f.read(65536)
- if not chunk:
- break
- result.extend(chunk)
- if limit is None:
- continue
- if exact:
- if len(result) > limit:
- raise DecompressionBombError(
- 'Decompressed data size of {} bytes exceeds the expected maximum of {} bytes; '
- 'the stream contains surplus data (possible corruption or decompression bomb, CWE-409).'
- .format(len(result), limit))
- else:
- if len(result) > 2 * limit:
- raise DecompressionBombError(
- 'Decompressed data size of {} bytes exceeds 2x the safety limit of {} bytes; '
- 'this looks like a decompression bomb (CWE-409). Set '
- 'json_tricks.utils.DECOMPRESSION_LIMIT to None to allow it.'.format(len(result), limit))
- if not warned and len(result) > limit:
- warnings.warn(
- 'Decompressed data size exceeds the safety limit of {} bytes; this may be a '
- 'decompression bomb (CWE-409). Set json_tricks.utils.DECOMPRESSION_LIMIT to None '
- 'to silence, or to a higher value to allow.'.format(limit), DecompressionBombWarning)
- warned = True
- return bytes(result)
+ with gzip.GzipFile(fileobj=io.BytesIO(data)) as fh:
+ if max_size is None:
+ return fh.read()
+ result = fh.read(max_size + 1)
+ if len(result) > max_size:
+ raise ValueError('gzip stream holds more than the expected {} bytes'.format(max_size))
+ return result
is_py3 = (version[:2] == '3.')
diff --git a/tests/test_decompression_bomb.py b/tests/test_decompression_bomb.py
deleted file mode 100644
index 9a5505c..0000000
--- a/tests/test_decompression_bomb.py
+++ /dev/null
@@ -1,217 +0,0 @@
-#!/usr/bin/env python
-# -*- coding: utf-8 -*-
-
-"""
-Tests for the decompression bomb protection (CWE-409).
-
-The limit is a module-level setting ``json_tricks.utils.DECOMPRESSION_LIMIT``
-(default ``None`` = no checking). When set to an int (bytes), decompression
-follows Pillow-style semantics: warn at 1x the limit, raise a dedicated
-``DecompressionBombError`` at 2x. The numpy ``b64.gz:`` path additionally
-imposes an exact cap of ``prod(shape) * itemsize`` bytes regardless of the
-limit, doubling as an integrity check.
-
-Tests use a tiny limit (a few KB) and tiny payloads so they run fast in CI.
-"""
-
-import gzip
-import io
-from base64 import standard_b64encode
-from warnings import catch_warnings, simplefilter
-
-import pytest
-from numpy import array, array_equal, zeros
-from pytest import raises, warns
-
-from json_tricks import dumps, loads
-from json_tricks import utils as _utils_mod
-from json_tricks.utils import (
- DecompressionBombError,
- DecompressionBombWarning,
- check_decompression_size,
- gzip_compress,
- gzip_decompress,
-)
-
-
-@pytest.fixture
-def reset_limit():
- """Reset DECOMPRESSION_LIMIT to None after each test so they don't pollute each other."""
- yield
- _utils_mod.DECOMPRESSION_LIMIT = None
-
-
-def _set_limit(n):
- """Set the single source of truth on the utils module."""
- _utils_mod.DECOMPRESSION_LIMIT = n
-
-
-def _gzip_bomb(raw_size, compresslevel=9):
- """Build a small gzip payload that decompresses to ``raw_size`` bytes of zeros."""
- buf = io.BytesIO()
- with gzip.GzipFile(fileobj=buf, mode='wb', compresslevel=compresslevel, mtime=0) as f:
- f.write(b'\x00' * raw_size)
- return buf.getvalue()
-
-
-# --------------------------------------------------------------------------- #
-# gzip_decompress: Pillow-style warn@1x / raise@2x
-# --------------------------------------------------------------------------- #
-
-def test_no_limit_default_allows_large_data():
- """With the default limit of None, large payloads decompress without warnings or errors."""
- raw = b'A' * (200 * 1024) # 200KB
- compressed = gzip_compress(raw, compresslevel=9)
- with catch_warnings():
- simplefilter('error') # any warning becomes an error
- out = gzip_decompress(compressed)
- assert out == raw
-
-
-def test_warn_at_1x_limit(reset_limit):
- _set_limit(8 * 1024) # 8KB limit
- # 10KB decompressed: exceeds 8KB limit but not 16KB (2x) -> warn only
- compressed = gzip_compress(b'B' * (10 * 1024), compresslevel=9)
- with warns(DecompressionBombWarning):
- out = gzip_decompress(compressed)
- assert len(out) == 10 * 1024
-
-
-def test_raise_at_2x_limit(reset_limit):
- _set_limit(8 * 1024) # 8KB limit
- # 20KB decompressed: exceeds 16KB (2x) -> raise
- compressed = gzip_compress(b'C' * (20 * 1024), compresslevel=9)
- with raises(DecompressionBombError):
- gzip_decompress(compressed)
-
-
-def test_explicit_max_size_override(reset_limit):
- """An explicit max_size takes precedence over the module global."""
- _set_limit(8 * 1024)
- raw = b'D' * (10 * 1024) # would warn under the 8KB global
- compressed = gzip_compress(raw, compresslevel=9)
- # With max_size=100KB, 10KB is fine (no warn, no raise)
- with catch_warnings():
- simplefilter('error')
- out = gzip_decompress(compressed, max_size=100 * 1024)
- assert out == raw
-
-
-def test_exact_mode_raises_on_surplus():
- """exact=True raises as soon as decompressed size exceeds max_size (no 2x buffer)."""
- raw = b'E' * (10 * 1024)
- compressed = gzip_compress(raw, compresslevel=9)
- # exact cap just below the actual size -> raise
- with raises(DecompressionBombError):
- gzip_decompress(compressed, max_size=10 * 1024 - 1, exact=True)
- # exact cap exactly at the actual size -> ok
- out = gzip_decompress(compressed, max_size=10 * 1024, exact=True)
- assert out == raw
-
-
-def test_empty_data():
- compressed = gzip_compress(b'', compresslevel=5)
- assert gzip_decompress(compressed) == b''
-
-
-# --------------------------------------------------------------------------- #
-# check_decompression_size helper
-# --------------------------------------------------------------------------- #
-
-def test_check_size_none_limit_noop(reset_limit):
- _set_limit(None)
- # any size, no warning, no error
- with catch_warnings():
- simplefilter('error')
- check_decompression_size(10 ** 12, 'big array')
-
-
-def test_check_size_warn_at_1x(reset_limit):
- _set_limit(1024)
- with warns(DecompressionBombWarning):
- check_decompression_size(2048, 'ctx') # 2x limit but not > 2x -> warn
-
-
-def test_check_size_raise_at_2x(reset_limit):
- _set_limit(1024)
- with raises(DecompressionBombError):
- check_decompression_size(2049, 'ctx') # > 2x -> raise
-
-
-# --------------------------------------------------------------------------- #
-# loads(): top-level auto-decompression path
-# --------------------------------------------------------------------------- #
-
-def test_loads_gzip_bomb_rejected_when_limit_set(reset_limit):
- _set_limit(8 * 1024)
- # A gzip bomb that decompresses to ~200KB (>> 2x limit)
- bomb = _gzip_bomb(200 * 1024)
- with raises(DecompressionBombError):
- loads(bomb)
-
-
-def test_loads_normal_gzip_unaffected_when_no_limit(reset_limit):
- _set_limit(None)
- original = dumps({'key': 'value', 'num': 42})
- compressed = gzip_compress(original.encode('utf-8'), compresslevel=5)
- result = loads(compressed)
- assert result == {'key': 'value', 'num': 42}
-
-
-# --------------------------------------------------------------------------- #
-# numpy b64.gz: ndarray path: exact cap + claimed-size check
-# --------------------------------------------------------------------------- #
-
-def _encode_gz_ndarray(arr):
- """Encode an ndarray to its b64.gz: compact form, bypassing the size heuristic
- that would otherwise skip compression for tiny arrays."""
- from json_tricks.utils import gzip_compress
- data = arr.tobytes()
- gz = gzip_compress(data, compresslevel=9)
- return 'b64.gz:' + standard_b64encode(gz).decode('ascii')
-
-
-def test_ndarray_normal_roundtrip_no_limit(reset_limit):
- _set_limit(None)
- arr = zeros((10, 10), dtype='float64') # 800 bytes
- txt = dumps({'__ndarray__': _encode_gz_ndarray(arr),
- 'dtype': 'float64', 'shape': (10, 10), 'Corder': True})
- out = loads(txt)
- assert array_equal(out, arr)
-
-
-def test_ndarray_exact_cap_rejects_surplus_bytes(reset_limit):
- """Decompressed stream larger than shape*itemsize -> DecompressionBombError (integrity check)."""
- _set_limit(None) # the exact cap is independent of the module limit
- # Claim shape (10,) float32 = 40 bytes, but the gzipped payload actually
- # decompresses to 200 bytes (zeros). The exact cap must reject it.
- real_data = b'\x00' * 200
- gz = gzip_compress(real_data, compresslevel=9)
- encoded = 'b64.gz:' + standard_b64encode(gz).decode('ascii')
- txt = dumps({'__ndarray__': encoded, 'dtype': 'float32', 'shape': (10,)})
- with raises(DecompressionBombError):
- loads(txt)
-
-
-def test_ndarray_claimed_size_raise_at_2x(reset_limit):
- """Claimed size (shape*itemsize) > 2x the limit -> raise before decompressing."""
- _set_limit(64) # 64 bytes
- # shape (100,) float64 -> 800 bytes claimed >> 2*64=128 -> raise
- arr = zeros(100, dtype='float64')
- txt = dumps({'__ndarray__': _encode_gz_ndarray(arr),
- 'dtype': 'float64', 'shape': (100,)})
- with raises(DecompressionBombError):
- loads(txt)
-
-
-def test_ndarray_claimed_size_warn_at_1x(reset_limit):
- """Claimed size > 1x but <= 2x the limit -> warn, but still decompress."""
- _set_limit(300) # 300 bytes
- # shape (50,) float32 -> 200 bytes claimed: 200 > 300? no -> no warn.
- # Use shape (80,) float32 -> 320 bytes: 320 > 300 (warn) but 320 <= 600 (no raise).
- arr = zeros(80, dtype='float32')
- txt = dumps({'__ndarray__': _encode_gz_ndarray(arr),
- 'dtype': 'float32', 'shape': (80,)})
- with warns(DecompressionBombWarning):
- out = loads(txt)
- assert array_equal(out, arr)
diff --git a/tests/test_np.py b/tests/test_np.py
index 66419d8..974cc04 100644
--- a/tests/test_np.py
+++ b/tests/test_np.py
@@ -1,13 +1,14 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-
+from base64 import standard_b64encode
from copy import deepcopy
from os.path import join
from tempfile import mkdtemp
import sys
from warnings import catch_warnings, simplefilter
-from pytest import warns
+from pytest import raises, warns
from numpy import arange, ones, array, array_equal, finfo, iinfo, pi
from numpy import int8, int16, int32, int64, uint8, uint16, uint32, uint64, \
float16, float32, float64, complex64, complex128, zeros, ndindex
@@ -17,7 +18,7 @@ from numpy.testing import assert_equal
from json_tricks import numpy_encode
from json_tricks.np import dump, dumps, load, loads
from json_tricks.np_utils import encode_scalars_inplace
-from json_tricks.utils import JsonTricksDeprecation, gzip_decompress
+from json_tricks.utils import JsonTricksDeprecation, gzip_compress, gzip_decompress
from .test_bare import cls_instance
from .test_class import MyTestCls
@@ -212,6 +213,25 @@ def test_dtype_object():
assert array_equal(back, arr)
+def test_dtype_object_shape_must_match_data():
+ # a declared shape must never size the allocation on its own
+ with raises(ValueError):
+ loads('{"__ndarray__": [], "dtype": "object", "shape": [100000000]}')
+ # only the first branch is nested deeply, so the declared 2**26 elements do not exist
+ node = 0
+ for _ in range(26):
+ node = [node, 0]
+ with raises(ValueError):
+ loads(dumps({'__ndarray__': node, 'dtype': 'object', 'shape': [2] * 26}))
+
+
+def test_compact_gzip_surplus_data_rejected():
+ # 1MB of zeros behind a shape that claims only 40
+ payload = standard_b64encode(gzip_compress(b'\x00' * (1 << 20), 9)).decode('ascii')
+ with raises(ValueError):
+ loads('{"__ndarray__": "b64.gz:%s", "dtype": "float32", "shape": [10]}' % payload)
+
+
def test_compact_mode_unspecified():
# Other tests may have raised deprecation warning, so reset the cache here
numpy_encode._warned_compact = FalseCan we also rebase here to pull in #113 for when we run the CI? |
9ea84ed to
4f8b988
Compare
|
Thanks for the suggestion @claydugo — done. I adopted your patch pretty much as-is and dropped the separate machinery.
Full suite passes ( |
Summary
gzip_decompressinjson_tricks/utils.pyread the entire decompressed stream into memory with no limit. Sinceloads()auto-detects gzip-compressed input via the\x1f\x8bmagic bytes, and the numpyb64.gz:path in_bin_str_to_ndarray(decoders.py) also decompresses, a small crafted payload can decompress to gigabytes, causing memory exhaustion (CWE-409).Additionally,
_lists_of_obj_to_ndarrayallocated an array from the declaredshapewithout checking that the nested data actually backs that shape, so a hostileshapecould size the allocation on its own.Fix
gzip_decompress(data, max_size=None)(utils.py): whenmax_sizeis given, read at mostmax_size + 1bytes and raiseValueErrorif the stream holds more. ANonelimit preserves existing behavior. This bounds memory whenever the expected size is known in advance._bin_str_to_ndarray(decoders.py): theb64.gz:branch now computes the exact expected byte count asitemsize * prod(shape)and passes it asmax_size, so a corrupt or hostile stream cannot expand beyond the declared array. A negativeexpected_bytes(overflow) is rejected._lists_of_obj_to_ndarray(decoders.py): the declaredshapeis now validated against the actual nested data before any allocation, so a hugeshapecannot size theempty()call on its own.Changes
json_tricks/utils.pygzip_decompressgains optionalmax_size(exact-cap read +ValueError)json_tricks/decoders.pyb64.gz:ndarray path passes exactmax_size;_lists_of_obj_to_ndarrayvalidates shape vs. datatests/test_np.pyTotal: 3 files, +48/-7.
Testing
All existing tests pass, plus 2 new tests in the preexisting
tests/test_np.py:test_dtype_object_shape_must_match_data— a declaredshapewith no/insufficient backing data raisesValueError.test_compact_gzip_surplus_data_rejected— 1MB of zeros behind ashapeclaiming 40 bytes raisesValueError.Rebased onto current master (
45b96ae, includes #113) and squashed to a single commit.