fix: address critical security vulnerabilities (path traversal, command injection, and more)#60
Open
19prince wants to merge 1 commit intogemini-cli-extensions:mainfrom
Conversation
- Path traversal (High): findInputFile() now rejects absolute paths and path traversal sequences (../), resolves paths and verifies they stay within the intended search root before any fs.readFile call - Command injection (High): openImagePreview() replaced exec() string interpolation with execFile() + argument array, removing shell involvement entirely - Input validation (Medium): added validateToolArgs() in index.ts enforcing non-empty prompt, 10k char limit, integer ranges for outputCount/steps (1-8/2-8), non-empty file param - Error message leakage (Low): replaced full filesystem search path disclosure with a generic not-found message - Model name allowlist (Low): NANOBANANA_MODEL validated against known model identifiers; falls back to default with warning if unknown - File size guard (Low): saveImageFromBase64() rejects API responses larger than 50 MB before writing to disk - Vulnerable dependencies: npm audit fix resolves 5 CVEs including ReDoS in @modelcontextprotocol/sdk, ajv, qs; DoS in body-parser; HMAC bypass in jws Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
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.
Summary
Security audit of the MCP server identified several vulnerabilities. This PR fixes all of them.
Critical / High
Path traversal → arbitrary file read (
fileHandler.ts):findInputFile()previously accepted absolute paths (e.g./etc/passwd,~/.ssh/id_rsa) and returned them verbatim forfs.readFile(), which then sent the contents to the Gemini API. Fix: reject absolute paths outright, reject..sequences, resolve the final path and assert it stays inside the intended search root.Command injection (
imageGenerator.ts):openImagePreview()built shell commands via string interpolation —open "${filePath}"— and passed them toexec(). A path containing"or backticks could break out of the quoting. Fix: replaced withexecFile()+ argument array, removing shell involvement entirely.Medium
index.ts): all tool arguments were accepted via bare TypeScript casts with no runtime enforcement. AddedvalidateToolArgs()enforcing: non-emptyprompt, 10 000 character limit,outputCountinteger 1–8,stepsinteger 2–8, non-emptyfilewith 255 char limit.Low
Error messages leaking filesystem layout (
imageGenerator.ts): "file not found" responses previously included the full list of searched paths (disclosingprocess.cwd(),$HOME, etc.). Replaced with a generic message.Unvalidated
NANOBANANA_MODELenv var (imageGenerator.ts): model name now checked against an allowlist of the three documented models; unknown values fall back to the default with a warning.Unbounded file write (
fileHandler.ts):saveImageFromBase64()now rejects API responses larger than 50 MB before writing to disk.Vulnerable dependencies:
npm audit fixresolves 5 CVEs — ReDoS in@modelcontextprotocol/sdk,ajv,qs; DoS inbody-parser; HMAC bypass injws.Test plan
npm run buildpasses cleanly inmcp-server/npm auditreports 0 vulnerabilitiesedit_imagewith an absolute path (e.g./etc/passwd) returns "File not found" rather than reading the fileedit_imagewith a../traversal path returns "File not found"generate_imagewith an empty prompt returns a validation errorgenerate_imagewithoutputCount: 99returns a validation error🤖 Generated with Claude Code