fix: validate genesis file type and size before reading (PM-19964)#832
Merged
Conversation
Contributor
|
KICS version: v2.1.19
|
e24ebb4 to
29eb70e
Compare
justinfrevert
approved these changes
Mar 5, 2026
Unbounded genesis file reads in Cfg::load_spec — security audit finding. See: https://shielded.atlassian.net/browse/PM-19964 Made-with: Cursor
Introduce validated_file module with safe_read() and safe_read_to_string() that validate file metadata before reading: - Reject symlinks via symlink_metadata() - Reject non-regular files (directories, devices, FIFOs) - Enforce configurable max file size (default 10 MB) Includes 14 unit tests covering happy paths, rejection paths, boundary conditions, and error message quality. Ref: PM-19964 Made-with: Cursor
Replace all 10 std::fs::read / std::fs::read_to_string calls in Cfg::load_spec() with validated_file::safe_read and safe_read_to_string. Files are now validated for type and size before reading. Ref: PM-19964 Made-with: Cursor
Replace std::fs::read_to_string with validated_file::safe_read_to_string in the filesystem fallback path of CfgPreset::load_config(). Built-in presets via get_config() are unaffected. Ref: PM-19964 Made-with: Cursor
Replace std::fs::read_to_string with validated_file::safe_read_to_string for node key file reading in SubstrateCfg::try_from(). Error mapped to sc_cli::Error::Input. Ref: PM-19964 Made-with: Cursor
Ref: PM-19964 Made-with: Cursor
Made-with: Cursor
… size limit Addresses L1 finding from test suite review — safe_read_to_string variants were implicitly covered via shared validate_file_metadata but lacked explicit boundary tests. Made-with: Cursor
Replace `|e| ConfigError::Message(e)` with `ConfigError::Message` and `|e| sc_cli::Error::Input(e)` with `sc_cli::Error::Input` to satisfy clippy::redundant_closure. Made-with: Cursor
Made-with: Cursor
48670b6 to
eb06e39
Compare
Fixes clippy::useless_vec on Rust 1.93 — use &[..] instead of &vec![..] for fixed-size test data in write_all calls. Ref: PM-19964 Made-with: Cursor
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
3 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
Validate genesis file type and size before reading to address Least Authority audit Issue AI (unbounded reads in
Cfg::load_spec). Introduces avalidated_filemodule that rejects symlinks, non-regular files, and files exceeding 10 MB across all 12 genesis/config file read sites in thecfgsubsystem.🎫 Ticket 📐 Engineering 🧪 Test Plan
Motivation
The Least Authority security audit (Issue AI) identified that
Cfg::load_specreads genesis and configuration files usingstd::fs::readandstd::fs::read_to_stringwithout validating file type or size. This allows potential denial-of-service through oversized files, symlink traversal to unintended paths, or non-file inputs (e.g., directories, device files).All file reads in the
cfgmodule occur during node startup and process operator-supplied configuration files, making pre-read validation a straightforward mitigation with no performance impact on normal operation.Changes
validated_filemodule — Newsafe_read()andsafe_read_to_string()functions that validate file metadata before reading: reject symlinks (viasymlink_metadata), reject non-regular files, enforce 10 MB size limit (MAX_GENESIS_FILE_SIZE)cfg/mod.rs— 10std::fs::read/read_to_stringcalls inload_specreplaced with validated variantsmeta_cfg/mod.rs— 1std::fs::read_to_stringcall inCfgPreset::load_configreplacedsubstrate_cfg/mod.rs— 1std::fs::read_to_stringcall inSubstrateCfg::try_fromreplacedcargo test -p midnight-node);cargo clippy -D warningsclean📌 Submission Checklist
changes/changed/validate-genesis-file-reads.md)🔱 Fork Strategy
🗹 TODO before merging