-
-
Notifications
You must be signed in to change notification settings - Fork 961
Document lowdb path traversal vulnerability #614
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,87 @@ | ||||||||||||||||||||||||||||||||
| <!-- GITHUB SECURITY ADVISORY SUBMISSION --> | ||||||||||||||||||||||||||||||||
| <!-- Title: Path Traversal via Unsanitized Filename in lowdb Adapter Constructor --> | ||||||||||||||||||||||||||||||||
| <!-- Severity: High --> | ||||||||||||||||||||||||||||||||
| <!-- Ecosystem: npm --> | ||||||||||||||||||||||||||||||||
| <!-- Package name: lowdb --> | ||||||||||||||||||||||||||||||||
| <!-- Affected versions: <= 7.0.1 --> | ||||||||||||||||||||||||||||||||
| <!-- Patched versions: (leave blank) --> | ||||||||||||||||||||||||||||||||
| <!-- CWE: CWE-22 --> | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ## Summary | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| lowdb v7.0.1's file-based adapter constructors (`TextFile`, `JSONFile`, `DataFile`) accept a `filename` parameter with zero path validation or sanitization. If any user input influences the database filename, an attacker can read or overwrite arbitrary files on the filesystem via directory traversal or absolute path injection. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ## Details | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| The `TextFile` adapter constructor (`lib/adapters/node/TextFile.js`, lines 8-10) stores the filename verbatim: | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| ```javascript | ||||||||||||||||||||||||||||||||
| constructor(filename) { | ||||||||||||||||||||||||||||||||
| this.#filename = filename; // Stored verbatim — no validation | ||||||||||||||||||||||||||||||||
| this.#writer = new Writer(filename); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
Comment on lines
+18
to
+22
|
||||||||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| There is no: | ||||||||||||||||||||||||||||||||
| - Path normalization or canonicalization | ||||||||||||||||||||||||||||||||
| - Rejection of `..` traversal sequences | ||||||||||||||||||||||||||||||||
| - Base-directory restriction | ||||||||||||||||||||||||||||||||
| - Rejection of absolute paths | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| The `JSONFilePreset` convenience function (`lib/presets/node.js`, line 4) passes the filename directly through: | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| The `JSONFilePreset` convenience function (`lib/presets/node.js`, line 4) passes the filename directly through: | |
| The `JSONFilePreset` convenience function (`src/presets/node.ts`) passes the filename directly through: |
Copilot
AI
Mar 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The statement about steno's Writer creating a .tmp file isn't verifiable from this repo (steno source isn't included), and lowdb itself already creates a .${basename}.tmp file in TextFileSync (src/adapters/node/TextFile.ts). Consider either citing lowdb's own temp-file behavior (sync adapter) or linking to the exact steno version/source to avoid an inaccurate claim.
Copilot
AI
Mar 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The PoC suggests that a JSON.parse error on a non-JSON file "may leak file contents". In lowdb's current implementation (DataFile.read() → JSON.parse), the thrown SyntaxError message doesn't include the full file contents by default; any disclosure would depend on how the host app logs/returns errors. Please reword to avoid overstating the direct impact (e.g., parsing error/DoS unless the app exposes the error or loaded JSON).
Copilot
AI
Mar 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This "Arbitrary file read" bullet appears to rely on JSON.parse failure error messages leaking file contents, which lowdb does not do directly. It would be more accurate to describe this as "arbitrary file access attempt" / "potential read if the targeted file is valid JSON and the application exposes db contents", or "error-triggered info leak depending on application error handling".
Copilot
AI
Mar 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The remediation snippet converts filename to str but then calls path.resolve(filename). In Node, path.resolve expects a string; passing a non-string PathLike (e.g., Buffer/URL) can throw. Use the already-derived str for all path.* operations, and consider a more robust traversal mitigation than str.includes('..') (e.g., normalize and enforce that the resolved path stays within an allowed base directory).
| const str = filename.toString(); | |
| if (str.includes('..') || path.isAbsolute(str)) { | |
| throw new Error('lowdb: path traversal detected'); | |
| } | |
| this.#filename = path.resolve(filename); | |
| const baseDir = path.resolve(process.cwd()); | |
| const str = filename.toString(); | |
| const resolvedPath = path.resolve(baseDir, str); | |
| // Ensure the resolved path stays within the allowed base directory | |
| if (!resolvedPath.startsWith(baseDir + path.sep)) { | |
| throw new Error('lowdb: path traversal detected'); | |
| } | |
| this.#filename = resolvedPath; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The advisory references
lib/adapters/node/TextFile.js(and specific line numbers), but this repository doesn't contain that path—only the TypeScript source exists undersrc/adapters/node/TextFile.ts. Consider updating the file references (and removing line numbers, or recalculating them against the actual files) so readers can verify the claim in this repo.