feat: Add Parse.File.url validation with config fileUpload.allowedFileUrlDomains against SSRF attacks#10044
Conversation
|
🚀 Thanks for opening this pull request! |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughAdds domain-whitelisting for file URLs: new FileUrlValidator validates File.url entries against fileUpload.allowedFileUrlDomains; config, options, docs, and types added; validation invoked in database create/update, GraphQL mutations, and cloud function routing; extensive tests added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant API as API (REST / GraphQL / CloudFunction)
participant Validator as FileUrlValidator
participant DB as DatabaseController
Client->>API: send create/update with object containing File.url
API->>Validator: validateFileUrlsInObject / validateFileUrl
alt validation passes
Validator-->>API: OK
API->>DB: persist object
DB-->>API: saved
API-->>Client: success response
else validation fails
Validator-->>API: throws Parse.Error (FILE_SAVE_ERROR)
API-->>Client: error response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10044 +/- ##
==========================================
+ Coverage 92.53% 92.55% +0.02%
==========================================
Files 190 191 +1
Lines 15521 15573 +52
Branches 176 176
==========================================
+ Hits 14362 14414 +52
Misses 1147 1147
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/Config.js`:
- Around line 553-557: The config allows fileUpload.allowedFileUrlDomains to be
an array but doesn't validate element types, which later causes a toLowerCase
TypeError in FileUrlValidator; update the validation where
fileUpload.allowedFileUrlDomains is set (near
FileUploadOptions.allowedFileUrlDomains.default) to ensure every entry is a
non-empty string (e.g., iterate the array and throw a clear error if any element
is not typeof 'string' or is an empty string) so callers get an immediate,
descriptive config error before FileUrlValidator runs.
In `@src/Options/index.js`:
- Around line 633-635: Add the missing allowedFileUrlDomains property to the
FileUploadOptions TypeScript interface in types/Options/index.d.ts so TypeScript
consumers see the option; declare it with the same shape used elsewhere (e.g.,
allowedFileUrlDomains?: string[]), matching how it appears in
src/Options/index.js, src/Options/Definitions.js, and src/Options/docs.js, and
run type checks to ensure no other type changes are required.
🧹 Nitpick comments (5)
spec/FileUrlValidator.spec.js (1)
75-83: Consider adding tests for IP-based URLs to strengthen SSRF coverage.The partial hostname match test is excellent for preventing domain suffix attacks. However, SSRF attacks commonly use IP addresses (e.g.,
http://127.0.0.1/file,http://169.254.169.254/metadata) to target internal services. Consider adding a test to verify that IP-based URLs are correctly handled whenallowedFileUrlDomainsonly contains domain names.src/GraphQL/transformers/mutation.js (1)
100-103: Inlinerequireinside hot path — consider hoisting to module scope.The
require('../../FileUrlValidator')is called on every file mutation. While Node.js cachesrequireresults, the repeated cache lookup and destructuring inside the function body is unnecessary. Both this file andFunctionsRouter.jsuse this pattern — consider hoisting the import to the top of the file for clarity and consistency with the existing ES module imports (line 1–4).Proposed fix
At the top of the file (after line 4):
import { validateFileUrl } from '../../FileUrlValidator';Then simplify lines 100–103:
if (file.url) { - const { validateFileUrl } = require('../../FileUrlValidator'); validateFileUrl(file.url, config); }spec/ParseGraphQLServer.spec.js (1)
10257-10258: Remove redundant schema cache clearing.Line 10257 already clears the schema cache via
resetGraphQLCache(), so Line 10258 is redundant and can be dropped for clarity.♻️ Proposed cleanup
- await resetGraphQLCache(); - await parseGraphQLServer.parseGraphQLSchema.schemaCache.clear(); + await resetGraphQLCache();src/Controllers/DatabaseController.js (2)
502-507: Prefer a top-levelimportover a dynamicrequireinside the hot path.Every other dependency in this file uses a static ES
import. The inlinerequire('../FileUrlValidator')re-resolves the module on everyupdate(andcreate) call. Move it to the top with the other imports:♻️ Suggested change
Add at the top of the file (e.g. after line 14):
import { validateFileUrlsInObject } from '../FileUrlValidator';Then simplify both call sites:
try { - const { validateFileUrlsInObject } = require('../FileUrlValidator'); validateFileUrlsInObject(update, this.options); } catch (error) {
502-507: Extract the shared validation + error-wrapping into a private helper to reduce duplication.The try/catch/require/reject block is duplicated verbatim in
update(lines 502-507) andcreate(lines 845-850). A small helper keeps both paths in sync:♻️ Example helper
_validateFileUrls(obj) { try { validateFileUrlsInObject(obj, this.options); } catch (error) { throw error instanceof Parse.Error ? error : new Parse.Error(Parse.Error.FILE_SAVE_ERROR, error.message || error); } }Then in both
updateandcreate:- try { - const { validateFileUrlsInObject } = require('../FileUrlValidator'); - validateFileUrlsInObject(update, this.options); - } catch (error) { - return Promise.reject(error instanceof Parse.Error ? error : new Parse.Error(Parse.Error.FILE_SAVE_ERROR, error.message || error)); - } + try { + this._validateFileUrls(update); + } catch (error) { + return Promise.reject(error); + }Also applies to: 845-850
mtrezza
changed the title
Parse.File.url validation with config fileUpload.allowedFileUrlDomainsParse.File.url validation with config fileUpload.allowedFileUrlDomains against SSRF attacks
# [9.3.0-alpha.3](9.3.0-alpha.2...9.3.0-alpha.3) (2026-02-07) ### Features * Add `Parse.File.url` validation with config `fileUpload.allowedFileUrlDomains` against SSRF attacks ([#10044](#10044)) ([4c9c948](4c9c948))
|
🎉 This change has been released in version 9.3.0-alpha.3 |
2 participants
Pull Request
Issue
A developer can introduce a SSRF vulnerability if an attacker passes a crafted
Parse.Filewith an arbitrary URL (e.g.,{__type: "File", name: "file.txt", url: "http://malicious.example.com"}) as a Cloud Function parameter or save it to the database in aParse.Objectfield of type Object or Array. When cloud code or clients callParse.File.getData(), the Parse JS SDK makes an HTTP request to the attacker-controlled URL, leaking the server/client IP and potentially accessing internal services.Note that this is not a vulnerability of Parse Server itself, as developers should not trust client-provided data they don't fully control, but instead validate it. This PR introduces a feature that allows to easily validate the
Parse.File.urlwhenever JSON data is instantiated asParse.File, specifically in the following scenarios:Parse.Fileas item inParse.Objectfield of type Array.Parse.Fileas key value inParse.Objectfield of type Object.Parse.Fileas parameter in a Cloud Function call.This is not a breaking change, as the default configuration in this PR allows any URL. However, the default is deprecated and Parse Server logs a warning about a future change of the default value.
Summary by CodeRabbit
New Features
Tests
Documentation
Chores