Fix: KerasFileEditor reject HDF5 virtual datasets#22976
Merged
hertschuh merged 2 commits intoMay 27, 2026
Conversation
KerasFileEditor._extract_weights_from_store reads each leaf dataset with a raw value[()]. It already rejects group-level ExternalLink/SoftLink (keras-team#22899) and Dataset external storage (keras-team#22057, the CVE-2026-1669 fix), but never checks value.is_virtual. An HDF5 Virtual Dataset is a HardLink with external=None, so it passes both guards, yet h5py resolves its source mapping on read and returns bytes from another HDF5 file the victim process can read. The leaked tensors surface via editor.weights_dict / weights_summary() and are re-emitted as plain datasets by editor.save(), reproducing the CVE-2026-1669 cross-file disclosure channel via virtual storage instead of links. saving_lib.safe_get_h5_dataset already rejects virtual datasets; this path did not route through it. Add the matching is_virtual check inline. Regression test covers the VDS case.
![gemini-code-assist[bot]](https://avatars.mcszy.xyz/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances the security of KerasFileEditor by explicitly rejecting HDF5 virtual datasets to prevent potential unauthorized file access. A corresponding test case was added to verify this behavior. The review feedback suggests improving the error message by including the path of the virtual dataset, ensuring the feedback is contextual and informative in accordance with the Keras API design guidelines.
Comment on lines
+533
to
+534
| if value.is_virtual: | ||
| raise ValueError("Not allowed: H5 file with virtual Dataset") |
Contributor
There was a problem hiding this comment.
While this mirrors the error message in saving_lib, it would be more helpful to include the path of the virtual dataset to provide better context to the user, especially in an interactive tool like KerasFileEditor. This aligns with the Keras API design guidelines regarding contextual error messages.
Suggested change
| if value.is_virtual: | |
| raise ValueError("Not allowed: H5 file with virtual Dataset") | |
| if value.is_virtual: | |
| raise ValueError( | |
| f"Not allowed: H5 file with virtual Dataset at {current_inner_path}" | |
| ) |
References
- Rule 140: Provide detailed feedback messages upon user error. Error messages should be contextual, informative, and actionable. (link)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #22976 +/- ##
==========================================
- Coverage 80.11% 79.48% -0.64%
==========================================
Files 464 464
Lines 68504 68515 +11
Branches 11252 11254 +2
==========================================
- Hits 54885 54459 -426
- Misses 10793 11243 +450
+ Partials 2826 2813 -13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hertschuh
requested changes
May 26, 2026
hertschuh
left a comment
hertschuh
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the adding this!
Some nitpicks:
Comment on lines
+145
to
+146
| def test_rejects_virtual_dataset(self): | ||
| target_fpath = os.path.join( |
Collaborator
There was a problem hiding this comment.
Nitpick: call self.get_temp_dir() once and keep a reference to it to create both H5 files in the same temp folder.
|
|
||
| def test_rejects_virtual_dataset(self): | ||
| target_fpath = os.path.join( | ||
| self.get_temp_dir(), "victim_private.weights.h5" |
Collaborator
There was a problem hiding this comment.
nitpick: don't call it victim, something more neutral like other.weights.h5.
| with h5py.File(target_fpath, "w") as f: | ||
| f.create_dataset("s", data=np.arange(5, dtype="float32")) | ||
|
|
||
| attacker_fpath = os.path.join( |
Collaborator
There was a problem hiding this comment.
nitpick: don't call it attacker, something more neutral, like virtual.
Comment on lines
+531
to
+532
| # Reject HDF5 virtual datasets, which read through to other files | ||
| # on access. Mirrors `saving_lib.safe_get_h5_dataset`. |
Collaborator
There was a problem hiding this comment.
nitpick: remove this comment, the context for this can be found via "Blame" and the PR description. This comment is not very useful outside of the context of this PR.
- file_editor.py: drop the explanatory comment and include current_inner_path in the virtual-dataset rejection message so the user sees which dataset triggered it (Rule 140 / contextual error). - file_editor_test.py: reuse a single self.get_temp_dir() for both fixture files; rename victim_private.weights.h5 -> other.weights.h5 and vds_payload.weights.h5 -> virtual.weights.h5 (and the matching *_fpath locals) to use neutral names.
google-ml-butler
Bot
added
kokoro:force-run
ready to pull
Contributor
Author
|
Hi @hertschuh, https://huntr.com/bounties/6a825962-b361-4952-9735-3a66952899f1 Since the fix is already in, would you mind validating the report on huntr when you get a moment? Thanks for your time! |
hertschuh
added a commit
that referenced
this pull request
Jun 25, 2026
* Version bump to 3.12.3 * Fix: KerasFileEditor eject HDF5 ExternalLink/SoftLink groups (#22899) * KerasFileEditor: reject HDF5 ExternalLink/SoftLink groups `_extract_weights_from_store` walked the HDF5 tree with raw subscripts (`data[key]`, `value["vars"]`). When the input file is attacker-supplied, either of those subscripts transparently follows an `h5py.ExternalLink` or `h5py.SoftLink` group into another HDF5 file on the host. The attacker-controlled payload can be as small as ~944 bytes and contains no Python code, so it bypasses any static-analysis-based defense. After the link is followed, `editor.weights_dict` is populated from the linked file. A subsequent `editor.save(...)` — part of the documented edit workflow — writes those tensors back out as regular datasets, turning the editor into a full cross-file disclosure / exfiltration channel for any HDF5 file (e.g. another user's weights checkpoint on a shared training cluster, an `~/.cache/huggingface/...` cached model, etc.) that the victim process can read. PR #22057 (the original CVE-2026-1669 fix) already added a dataset-level `value.external` check to this same method, but only covered `Dataset` external storage. The matching `ExternalLink` / `SoftLink` guard on `Group` children — which is what `safe_get_h5_group` in `saving_lib` was written to provide — was not applied here. Apply it now: * Before subscripting `data[key]`, fetch the link class via `data.get(..., getclass=True, getlink=True)` and raise `ValueError` on `ExternalLink` / `SoftLink`. This mirrors `safe_get_h5_group` but handles the case where the child can be a `Dataset` rather than a `Group`. * Resolve `value["vars"]` through `saving_lib.safe_get_h5_group` so a nested link named `vars` cannot be used to redirect the editor. Keras's own `save_model` / `save_weights` never emits `ExternalLink` or `SoftLink`, so legitimate weight files are unaffected — only attacker- crafted payloads are rejected. Two regression tests cover both link kinds. Reported via huntr: https://huntr.com/bounties/d5688f27-73af-4853-8855-0c3a45c0f290 * KerasFileEditor: trim review comments Shorten the ExternalLink/SoftLink guard comment, restore the original "vars" recursion comment, and drop the added test docstrings/comments per review. No behavior change. * Fix: load_weights reject HDF5 ExternalLink/SoftLink on legacy .h5 dispatcher (#22900) * load_weights: route legacy `.h5` `model_weights` through safe_get_h5_group `saving_api.py` was the only remaining raw `f["model_weights"]` subscript on the legacy HDF5 weight-loading path. h5py transparently follows `h5py.ExternalLink` / `h5py.SoftLink` on plain subscripts, so a ~1 KB attacker-supplied `.h5` whose `model_weights` group is an `ExternalLink` redirects `keras.saving.load_weights(model, "x.h5")` (equivalently `model.load_weights("x.h5")`) into any HDF5 file the victim process can read. The contents land in the victim's in-process model and become trivially exfiltrable via `model.get_weights()`, `model.save(...)`, or inference. This is the same vulnerability class as CVE-2026-1669 and is a missed spot relative to PR #22057 and #22801, which already routed the equivalent accesses in `saving_lib.py`, `file_editor.py`, and `legacy/saving/legacy_h5_format.py` through `safe_get_h5_group` (introduced specifically for this purpose). `safe_mode` does not gate this path — it only guards Python-object deserialization. The fix is a one-line change here: use `saving_lib.safe_get_h5_group` at this dispatcher access, matching what `legacy_h5_format` already does for its own `f["model_weights"]` read. Keras's own `save_model` / `save_weights` never emits `ExternalLink` / `SoftLink`, so legitimate `.h5` weight files are unaffected; only attacker-crafted payloads are rejected. Reported via huntr: https://huntr.com/bounties/d5688f27-73af-4853-8855-0c3a45c0f290 * load_weights: trim review comments Remove the safe_get_h5_group dispatcher comment and the added test docstrings/comments per review. No behavior change. * Fix: reject hard-link tar members whose name escapes the extraction directory (#22973) * Reject hard links whose name escapes the extraction directory filter_safe_tarinfos / is_link_in_dir validated only the target (info.linkname) of a hard-link tar member, never the link's own destination (info.name). A crafted tar with a hard-link member whose name is an absolute path or contains "../" therefore passed the filter and was materialized outside the extraction directory by tarfile.extractall, allowing arbitrary file write/overwrite via keras.utils.get_file(extract=True) / extract_archive on Python < 3.12 (where tarfile's data filter is not applied). Validate info.name for hard links the same way regular files and symbolic links are already validated, completing the tar-extraction path-traversal hardening for the hard-link case. * Address review: shorten hard-link comment and drop redundant test Condense the is_link_in_dir hard-link comment to a single line per reviewer request; context lives in the PR description and blame. Remove FilterSafePathsTest.test_hardlink_name_traversal_skipped: the hardlink_name_out parameterized case in IsLinkInDirTest already covers rejection of a hard link whose name escapes base_dir. * Fix: reject HDF5 shape-bomb datasets in the main load_model/load_weights path (#22975) * Reject HDF5 datasets that declare far more data than is stored safe_get_h5_dataset materialized an h5py dataset sized to its declared shape without checking how much data is actually stored on disk. A crafted .keras/.weights.h5 with a chunked, compressed, fill-value-only dataset can declare an enormous shape (e.g. petabytes) while occupying a few kilobytes, forcing load_model/load_weights to attempt a huge allocation and OOM-kill the process (CWE-789 / CWE-409). Reject datasets whose declared in-memory size both exceeds a floor and exceeds the on-disk storage size by a large factor. Genuine arrays (even compressed) stay well within this bound; shape/decompression bombs do not. This extends the KerasFileEditor-only size guard to the main load_model/load_weights path. * Address review: 4 GiB floor, readable sizes, drop redundant test - Raise _H5_DATASET_BOMB_FLOOR_BYTES from 1 GiB to 4 GiB for consistency with the KerasFileEditor fix (PR #21880). - Format the declared/stored byte counts in the error with readable_memory_size so PiB-scale values are legible. - Remove SafeGetH5DatasetTest.test_allows_regular_dataset; normal dataset loading is already covered by the existing round-trip tests. * Fix: KerasFileEditor reject HDF5 virtual datasets (#22976) * Fix: KerasFileEditor reject HDF5 virtual datasets KerasFileEditor._extract_weights_from_store reads each leaf dataset with a raw value[()]. It already rejects group-level ExternalLink/SoftLink (#22899) and Dataset external storage (#22057, the CVE-2026-1669 fix), but never checks value.is_virtual. An HDF5 Virtual Dataset is a HardLink with external=None, so it passes both guards, yet h5py resolves its source mapping on read and returns bytes from another HDF5 file the victim process can read. The leaked tensors surface via editor.weights_dict / weights_summary() and are re-emitted as plain datasets by editor.save(), reproducing the CVE-2026-1669 cross-file disclosure channel via virtual storage instead of links. saving_lib.safe_get_h5_dataset already rejects virtual datasets; this path did not route through it. Add the matching is_virtual check inline. Regression test covers the VDS case. * Address review comments - file_editor.py: drop the explanatory comment and include current_inner_path in the virtual-dataset rejection message so the user sees which dataset triggered it (Rule 140 / contextual error). - file_editor_test.py: reuse a single self.get_temp_dir() for both fixture files; rename victim_private.weights.h5 -> other.weights.h5 and vds_payload.weights.h5 -> virtual.weights.h5 (and the matching *_fpath locals) to use neutral names. * Reject ZIP archive members that declare far more data than is stored (#23010) `_load_model_from_fileobj` reads `config.json`, `model.weights.h5` and the shard map from the `.keras` archive without bounding the decompressed size. The `config.json` read is the first thing load_model does, before any other validation and regardless of `safe_mode`. A crafted `.keras` can store a few KB on disk yet declare a multi-gigabyte member, so loading it allocates memory (or extracts to disk) for the declared size and the process is OOM-killed or fills the disk (CWE-409). Add `_reject_zip_bomb` (rejects a member whose declared size both exceeds a 4 GiB floor -- matching the HDF5 dataset guard for CVE-2026-0897 -- and exceeds its on-disk size by more than 100x) and a `_safe_zip_read` that checks then reads. Apply it to: - `config.json` and the shard map, via `_safe_zip_read`; - `model.weights.h5`, via `_reject_zip_bomb` placed *before* the in-memory/extract-to-disk/on-the-fly `try/except` -- the bare `except` falls back to reading the weights on the fly, so a check inside it would be swallowed and the bomb still read; checking up front covers all three paths. Keras writes archive members uncompressed (ratio ~1:1) and DEFLATE cannot exceed ~1032:1, so genuine artifacts -- including arbitrarily large legitimate models -- are unaffected. Co-authored-by: hertschuh <1091026+hertschuh@users.noreply.github.com> * Resolve paths with realpath to prevent symlink traversal in extraction (#23015) `resolve_path` wrapped `realpath` with `abspath`, which collapses `..` lexically before symlinks are resolved. For an archive member named `link/../x`, the `..` cancels the `link` component as text, so `realpath` never follows the symlink; `resolve_sub_path` then judges the member to be inside the extraction directory while `tarfile.extractall` writes it through the real symlink to a location outside it. A TAR with an in-bounds symlink `link -> .` plus a file `link/../<name>` therefore writes `<name>` outside the extraction directory (chainable to deeper paths) via `get_file(extract=True)` / `extract_archive`. Use `os.path.realpath(path)` directly: it returns an absolute path and follows symlinks component by component, so `..` is applied after the symlink is resolved and the containment check is correct. * Reject npz weight members that declare far more data than is stored (#23016) `NpzIOStore.get` returns `self.contents[path].tolist()`. Accessing `self.contents[path]` makes NumPy allocate an array sized to the `.npy` header's declared shape before any stored data is validated, so a tiny npz member can declare an enormous shape and drive an unbounded allocation. `keras.saving.load_model` selects the npz reader purely from the archive file list (`model.weights.npz` present and `model.weights.h5` absent), so an attacker who supplies the `.keras` file controls this path: a ~2 KB file can force an arbitrarily large allocation (clean `MemoryError`, or an OOM kill for a shape tuned near available RAM whose zero-filled member compresses heavily). `safe_mode` does not gate weight I/O. Peek each member's `.npy` header and reject it when the declared array size exceeds both a 4 GiB floor and 100x the bytes actually stored, before the array is materialized. Co-authored-by: hertschuh <1091026+hertschuh@users.noreply.github.com> * Validate DiskIOStore asset paths stay within the working directory (#23017) `DiskIOStore.make`/`get` joined a caller-supplied asset path onto the working directory (and `make` created it) without verifying that it stays inside that directory. This is the one asset I/O path in the save/load pipeline that skips the containment already applied to archive extraction (`filter_safe_tarinfos`/`filter_safe_zipinfos`) and Orbax asset writes (`_write_nested_dict_to_dir`), both of which use `resolve_sub_path`. Apply the same `resolve_sub_path` check in `make`/`get`/`has_path` so an asset path that escapes the working directory is rejected. Normal nested asset paths are unaffected. * 🛡️ Sentinel: [HIGH] Fix insecure deserialization in dataset utilities (#23026) * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * security: set allow_pickle=False for np.load in dataset utilities np.load defaults to allow_pickle=False for security reasons, to prevent insecure deserialization vulnerabilities that could lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. Several datasets in `keras/src/datasets` used `allow_pickle=True` unnecessarily for numeric arrays (e.g. `mnist`, `boston_housing`, `california_housing`). This commit removes the explicit `allow_pickle=True` argument for those datasets, enhancing the security of the application. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` (#23034) * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> * 🛡️ Sentinel: [ENHANCEMENT] Explicitly disable pickle in `np.load` 🚨 Severity: LOW / ENHANCEMENT 💡 Vulnerability: `np.load` was called without explicitly specifying `allow_pickle=False`. While modern NumPy versions default to `False`, relying on defaults can lead to insecure deserialization if the environment uses an older version or the default changes. 🎯 Impact: Insecure deserialization can lead to arbitrary code execution if a maliciously crafted `.npz` file is loaded. 🔧 Fix: Explicitly pass `allow_pickle=False` to `np.load` in `keras/src/saving/saving_lib.py`. ✅ Verification: Ran `pytest keras/src/saving/saving_lib_test.py` to ensure no regressions. Co-authored-by: rni418 <70032655+rni418@users.noreply.github.com> --------- Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * Make Lambda and TorchModuleWrapper from_config fail closed when safe_mode is unset (#23048) * Make Lambda and TorchModuleWrapper from_config fail closed when safe_mode is unset `Lambda.from_config` computed `safe_mode = safe_mode or in_safe_mode()`, which is `None` (falsy) when the layer is deserialized outside of a `SafeModeScope` and without an explicit `safe_mode` -- for example a direct `keras.Model.from_config(...)` / `keras.Sequential.from_config(...)` call, or a layer config that omits the `"module"` key and is routed through the legacy deserializer. In that case the guard did not fire and the marshalled lambda bytecode was loaded via `func_load`, allowing arbitrary code execution. `TorchModuleWrapper.from_config` had the same problem with its `torch.load()` pickle (`if in_safe_mode():`). Treat an unset safe mode as safe (fail closed), matching the handling already used by `TFSMLayer.from_config`: deserialization is refused unless safe mode is explicitly disabled via `safe_mode=False`, `SafeModeScope(False)`, or `keras.config.enable_unsafe_deserialization()`. `load_model` and `model_from_json` are unaffected -- they already establish a `SafeModeScope`. Adds regression tests for both layers and updates the `TorchModuleWrapper` round-trip test to disable safe mode explicitly. * Add `safe_mode` argument to `TorchModuleWrapper.from_config` Follow the `Lambda.from_config` / `TFSMLayer.from_config` pattern: accept `custom_objects` and `safe_mode` keyword arguments. When `safe_mode` is unset, fall back to the ambient `SafeModeScope`, treating an unset scope as safe (fail closed). Update the test to exercise the explicit `safe_mode=False` argument alongside the ambient-scope path. * Use `filter="data"` in `TarFile.extractall` on supported versions of Python 3.10 and 3.11. (#23108) While the security feature of `filter="data"` was introduced in Python 3.12, it was also backported to Python >= 3.10.12 and Python >= 3.11.4. * Add to the list of APIs that should not be part of a reloaded model. (#23115) Some APIs make no sense in the context of reloading a model and present a security risk. Renamed the set and generalized the error message. * Fix saving tests for r3.12 compatibility * remove the unrelated test in this branch --------- Co-authored-by: Ziyu Lin <104151270+LinZiyuu@users.noreply.github.com> Co-authored-by: hertschuh <1091026+hertschuh@users.noreply.github.com> Co-authored-by: Rakesh Iyer <70032655+rni418@users.noreply.github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
KerasFileEditor._extract_weights_from_storewalks the HDF5 tree and readseach leaf dataset with a raw
value[()]. PR #22899 added rejection ofgroup-level
ExternalLink/SoftLink, and #22057 (the CVE-2026-1669 fix)added a
Dataset.externalcheck to this same method — but neither rejectsvirtual datasets. An HDF5 Virtual Dataset (VDS) presents as an
h5py.HardLinkwithexternal == None, so it passes both existing guards;h5py then transparently resolves the VDS source mapping on
value[()]andreturns bytes from another HDF5 file on the host.
A small attacker
.weights.h5containing only a VDS leaf (under the usual<layer>/vars/<n>structure the editor walks) therefore redirects the editorinto any HDF5 file the victim process can read. The leaked tensors appear in
editor.weights_dict/weights_summary(), andeditor.save(...)re-emitsthem as ordinary datasets — the same cross-file disclosure / exfiltration
channel as CVE-2026-1669, via virtual storage instead of links.
safe_modeis irrelevant here: the payload contains no Python and the editorperforms no object deserialization.
Fix
saving_lib.safe_get_h5_datasetalready rejects virtual datasets, but thispath does not route through it. Add the matching
value.is_virtualcheckinline, right after the existing
value.externalguard. Keras's ownsave_weightsnever emits virtual datasets, so legitimate files areunaffected.
Test
test_rejects_virtual_datasetbuilds a VDS payload pointing at a separatefile and asserts
KerasFileEditor(...)raises.References
Contributor agreement