Repository navigation
fix(store): harden DB open against path injection (CodeQL #15) - #147
Merged
Merged
Conversation
Validate DB paths before create_dir_all/SQLite open (no '..', NUL, or URI names), drop SQLITE_OPEN_URI, and confine project ids to a hashed hex component. Fixes CodeQL rust/path-injection alert #15. Co-authored-by: Josh Boys <josh@arkahna.io>
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
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.
Closes CodeQL alert #15 (
rust/path-injection, high): uncontrolled data used in path expression atcrates/kindling-store/src/db.rscreate_dir_all.Investigation
This is a real sink (
create_dir_all+ SQLite open on a caller-supplied path) with a narrow threat model, not a classic remote traversal ofX-Kindling-Project.open_databaseasX-Kindling-Projectproject_db_path(home, root)→home/projects/<12-hex SHA-256>/kindling.db--db/KINDLING_DB_PATHSqliteKindlingStore::open/KindlingService::openTwo issues were not accepted residual:
SQLITE_OPEN_URI, andOpenFlags::default()includes URI as well. A path likefile:/tmp/x?mode=rwccould be reinterpreted as a URI instead of a file path.create_dir_allbefore validation. The CLI (open_service,init --skip-db) mkdir'd parents from the raw path, so a..value could create directories even if open later failed.Confining every open to
~/.kindlingwould break legitimate--db,KINDLING_DB_PATH, and temp-dir tests. The store therefore still accepts any local filesystem location without../ URI / NUL.Fix
validate_db_pathruns before mkdir or SQLite open. Rejects empty / non-UTF-8 / NUL, SQLite special names (file:…,:memory:,:temp:), and..(the CodeQLDotDotChecksanitizer guard). The path used at the sink is reconstructed from the checked string.SQLITE_OPEN_URIand addSQLITE_OPEN_NOFOLLOW.project_db_pathdocuments anddebug_asserts thatproject_idis a single 12-hex component.init --skip-dbvalidates first.Residual
An operator can still open/create a DB at any path that does not contain
..(e.g.--db /var/tmp/kindling.db). That is intentional local configuration, not remote input. Daemon project routing remains hash-confined underkindling_home.Tests
.., URI/special names, empty, and NUL rejected.project_rootvalues stay underkindling_homeasprojects/<hex>/kindling.db.