fix: path traversal, arbitrary file read, and weak session secret in Node server#1387
Open
mohammadmseet-hue wants to merge 1 commit into
Open
Conversation
…cret 1. Path traversal via WebSocket template-config: Validate templateid and pluginid parameters against a strict allowlist pattern (alphanumeric, hyphens, underscores only) before using them to construct file paths in pushConfiguration(). Previously, values like "../../etc" could traverse outside the templates directory via fs.readFileSync. 2. Arbitrary file read via POST /updateTheme: Validate that the uiThemePath resolves within the allowed themes directory and has a .json extension before passing it to extractUITheme(), which uses require() to load the file. This prevents reading arbitrary server files and exposing their contents at GET /ui-theme.js. 3. Hardcoded session secret: The development config ships with "sample-secret-key-for-encryption" as session.secret.key. If this default value is used in production, session tokens become predictable. Generate a cryptographically random fallback secret at process startup when no strong key is configured.
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
This PR addresses three security vulnerabilities in the CDAP UI Node.js server:
1. Path Traversal via WebSocket
template-configMessage (High)File:
server/aggregator.js—pushConfiguration()The
templateidandpluginidparameters from WebSocket messages are concatenated directly into file paths and read withfs.readFileSync()without any validation:An attacker with a valid session token can send
templateid: "../../etc"andpluginid: "passwd"to read arbitrary.json-parseable files from the server filesystem. The file contents are returned through the WebSocket connection in the response payload.Fix: Validate both
templateidandpluginidagainst a strict allowlist pattern (/^[a-zA-Z0-9_-]+$/) before constructing file paths. Reject requests with a 400 status if validation fails.2. Arbitrary File Read via POST
/updateTheme(High)Files:
server/express.js,server/uiThemeWrapper.jsThe
/updateThemeendpoint accepts auiThemePathin the request body and passes it toextractUITheme(), which usesrequire()(via__non_webpack_require__) to load the file. The loaded content is then exposed atGET /ui-theme.jsas a JavaScript global:An attacker with a session token can load arbitrary files (
.js,.json,.node) from the server, and their contents become publicly readable at/ui-theme.js.Fix: Validate that the resolved theme path is within the allowed themes directory (
server/config/themes/) and has a.jsonextension before loading.3. Hardcoded Session Secret (Medium)
Files:
server/token.mjs,server/config/development/cdap.jsonThe development configuration ships with
"session.secret.key": "sample-secret-key-for-encryption". If this value is not overridden in production, all session tokens become predictable — any attacker who knows this default can forge valid session tokens and bypass session validation on all protected endpoints.Fix: Detect the insecure default value at startup and replace it with a cryptographically random secret generated via
crypto.randomBytes(). Log a warning to prompt operators to configure a proper secret.Related
Test Plan
template-configrequests with valid templateid/pluginid still worktemplate-configrequests with../in templateid or pluginid are rejected with 400POST /updateThemewith a path insideserver/config/themes/still worksPOST /updateThemewith../../etc/passwdor/etc/passwdis rejected with 400