diff --git a/src/context/directory/handlers/actionModules.ts b/src/context/directory/handlers/actionModules.ts index 09c06c9ad..3fd79adc0 100644 --- a/src/context/directory/handlers/actionModules.ts +++ b/src/context/directory/handlers/actionModules.ts @@ -30,11 +30,7 @@ function parse(context: DirectoryContext): ParsedActionModules { // The `module.code` can be a file path. It needs to be loaded. // It can be a relative path, so we need to handle both cases. const unixPath = module.code.replace(/[\\/]+/g, '/').replace(/^([a-zA-Z]+:|\.\/)/, ''); - if (fs.existsSync(unixPath)) { - module.code = context.loadFile(unixPath, moduleFolder); - } else { - module.code = context.loadFile(path.join(context.filePath, module.code), moduleFolder); - } + module.code = context.loadFile(unixPath, moduleFolder); } return module; diff --git a/src/context/directory/handlers/actions.ts b/src/context/directory/handlers/actions.ts index 3396cbba0..ff71244d3 100644 --- a/src/context/directory/handlers/actions.ts +++ b/src/context/directory/handlers/actions.ts @@ -30,13 +30,7 @@ function parse(context: DirectoryContext): ParsedActions { if (action.code) { // Convert `action.code` path to Unix-style path by replacing backslashes and multiple slashes with a single forward slash, and remove leading drive letters or './'. const unixPath = action.code.replace(/[\\/]+/g, '/').replace(/^([a-zA-Z]+:|\.\/)/, ''); - if (fs.existsSync(unixPath)) { - // If the Unix-style path exists, load the file from that path - action.code = context.loadFile(unixPath, actionFolder); - } else { - // Otherwise, load the file from the context's file path - action.code = context.loadFile(path.join(context.filePath, action.code), actionFolder); - } + action.code = context.loadFile(unixPath, actionFolder); } return action; diff --git a/src/context/directory/handlers/databases.ts b/src/context/directory/handlers/databases.ts index 69d611a75..309c3135e 100644 --- a/src/context/directory/handlers/databases.ts +++ b/src/context/directory/handlers/databases.ts @@ -33,6 +33,7 @@ type DatabaseMetadata = { function getDatabase( folder: string, + configRoot: string, mappingOpts: { mappings: KeywordMappings; disableKeywordReplacement: boolean } ): {} { const metaFile = path.join(folder, 'database.json'); @@ -68,10 +69,14 @@ function getDatabase( // skip invalid keys in customScripts object log.warn('Skipping invalid database configuration: ' + name); } else { - database.options.customScripts[name] = loadFileAndReplaceKeywords( - path.join(folder, script), - mappingOpts - ); + const resolvedBase = path.resolve(configRoot); + const toLoad = path.resolve(folder, script); + if (!toLoad.startsWith(resolvedBase + path.sep)) { + throw new Error( + `File reference "${script}" in database custom script "${name}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.` + ); + } + database.options.customScripts[name] = loadFileAndReplaceKeywords(toLoad, mappingOpts); } }); } @@ -90,7 +95,7 @@ function parse(context: DirectoryContext): ParsedDatabases { const databases = folders .map((f) => - getDatabase(f, { + getDatabase(f, context.filePath, { mappings: context.mappings, disableKeywordReplacement: context.disableKeywordReplacement, }) diff --git a/src/context/directory/index.ts b/src/context/directory/index.ts index 71eff04df..9eab34375 100644 --- a/src/context/directory/index.ts +++ b/src/context/directory/index.ts @@ -47,12 +47,17 @@ export default class DirectoryContext { } loadFile(f: string, folder: string) { - const basePath = path.join(this.filePath, folder); - let toLoad = path.join(basePath, f); - if (!isFile(toLoad)) { - // try load not relative to yaml file - toLoad = f; + const resolvedBase = path.resolve(this.filePath); + const inSubfolder = path.resolve(this.filePath, folder, f); + const inRoot = path.resolve(this.filePath, f); + const toLoad = isFile(inSubfolder) ? inSubfolder : inRoot; + + if (!toLoad.startsWith(resolvedBase + path.sep)) { + throw new Error( + `File reference "${f}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.` + ); } + return loadFileAndReplaceKeywords(toLoad, { mappings: this.mappings, disableKeywordReplacement: this.disableKeywordReplacement, diff --git a/src/context/yaml/index.ts b/src/context/yaml/index.ts index 24dc683bf..d73727a45 100644 --- a/src/context/yaml/index.ts +++ b/src/context/yaml/index.ts @@ -11,7 +11,7 @@ import { import pagedClient from '../../tools/auth0/client'; import log from '../../logger'; -import { isFile, toConfigFn, stripIdentifiers, formatResults, recordsSorter } from '../../utils'; +import { toConfigFn, stripIdentifiers, formatResults, recordsSorter } from '../../utils'; import handlers, { YAMLHandler } from './handlers'; import cleanAssets from '../../readonly'; import { Assets, Config, Auth0APIClient, AssetTypes, KeywordMappings } from '../../types'; @@ -58,12 +58,16 @@ export default class YAMLContext { } loadFile(f) { - let toLoad = path.join(this.basePath, f); - if (!isFile(toLoad)) { - // try load not relative to yaml file - toLoad = f; + const resolvedBase = path.resolve(this.basePath); + const toLoad = path.resolve(this.basePath, f); + + if (!toLoad.startsWith(resolvedBase + path.sep)) { + throw new Error( + `File reference "${f}" must be relative to the config directory. Absolute paths and paths outside the config root are not supported.` + ); } - return loadFileAndReplaceKeywords(path.resolve(toLoad), { + + return loadFileAndReplaceKeywords(toLoad, { mappings: this.mappings, disableKeywordReplacement: this.disableKeywordReplacement, }); diff --git a/test/context/directory/actions.test.js b/test/context/directory/actions.test.js index 976d693af..5b26310c8 100644 --- a/test/context/directory/actions.test.js +++ b/test/context/directory/actions.test.js @@ -15,7 +15,7 @@ const actionFiles = { '/** @type {PostLoginAction} */ module.exports = async (event, context) => { console.log(@@replace@@); return {}; };', 'action-one.json': `{ "name": "action-one", - "code": "./local/testData/directory/test1/actions/code.js", + "code": "./actions/code.js", "runtime": "node12", "dependencies": [ { @@ -42,7 +42,7 @@ const actionFilesWin32 = { '/** @type {PostLoginAction} */ module.exports = async (event, context) => { console.log(@@replace@@); return {}; };', 'action-one.json': `{ "name": "action-one", - "code": "local\\\\testData\\\\directory\\\\test1\\\\actions\\\\code.js", + "code": "actions\\\\code.js", "runtime": "node12", "dependencies": [ { @@ -150,6 +150,40 @@ describe('#directory context actions', () => { expect(context.assets.actions).to.deep.equal(actionsTarget); }); + it('should reject action code with a path outside the config directory', async () => { + const repoDir = path.join(testDataDir, 'directory', 'test-path-traversal'); + createDir(repoDir, { + [constants.ACTIONS_DIRECTORY]: { + 'action-one.json': JSON.stringify({ + name: 'action-one', + code: '/absolute/path/outside/config/code.js', + runtime: 'node12', + dependencies: [], + secrets: [], + supported_triggers: [{ id: 'post-login', version: 'v1' }], + deployed: true, + }), + }, + }); + const context = new Context({ AUTH0_INPUT_FILE: repoDir }, mockMgmtClient()); + await expect(context.loadAssetsFromLocal()).to.be.eventually.rejectedWith( + Error, + 'must be relative to the config directory' + ); + }); + + it('should accept action code with a path relative to the config directory', async () => { + const repoDir = path.join(testDataDir, 'directory', 'test1'); + createDir(repoDir, actionFiles); + const config = { + AUTH0_INPUT_FILE: repoDir, + AUTH0_KEYWORD_REPLACE_MAPPINGS: { replace: 'test-action' }, + }; + const context = new Context(config, mockMgmtClient()); + await context.loadAssetsFromLocal(); + expect(context.assets.actions).to.deep.equal(actionsTarget); + }); + it('should ignore bad actions directory', async () => { const repoDir = path.join(testDataDir, 'directory', 'test2'); cleanThenMkdir(repoDir); diff --git a/test/context/yaml/pages.test.js b/test/context/yaml/pages.test.js index a41852d21..a2eb1c2c2 100644 --- a/test/context/yaml/pages.test.js +++ b/test/context/yaml/pages.test.js @@ -25,24 +25,22 @@ describe('#YAML context pages', () => { cleanThenMkdir(dir); const htmlContext = 'this is a ##val1## @@val2@@'; - const htmlFile = path.join(testDataDir, 'page.html'); - fs.writeFileSync(htmlFile, htmlContext); const errorPageUrl = 'https://example.com'; const yaml = ` pages: - name: "login" - html: "${htmlFile}" + html: "page.html" - name: "password_reset" - html: "${htmlFile}" + html: "page.html" - name: "guardian_multifactor" - html: "${htmlFile}" + html: "page.html" enabled: false - name: "error_page" - html: "${htmlFile}" + html: "page.html" url: "${errorPageUrl}" show_log_link: false `; @@ -70,6 +68,8 @@ describe('#YAML context pages', () => { }, ]; createPagesDir(dir, target); + // Write page.html after createPagesDir since createPagesDir calls cleanThenMkdir(dir) + fs.writeFileSync(path.join(dir, 'page.html'), htmlContext); const yamlFile = path.join(dir, 'rule1.yaml'); fs.writeFileSync(yamlFile, yaml); @@ -83,6 +83,44 @@ describe('#YAML context pages', () => { expect(context.assets.pages).to.deep.equal(target); }); + it('should reject page html with a path outside the config directory', async () => { + const dir = path.join(testDataDir, 'yaml', 'pages-path-traversal'); + cleanThenMkdir(dir); + + const yaml = ` + pages: + - name: "login" + html: "/absolute/path/outside/config/login.html" + `; + const yamlFile = path.join(dir, 'config.yaml'); + fs.writeFileSync(yamlFile, yaml); + + const context = new Context({ AUTH0_INPUT_FILE: yamlFile }, mockMgmtClient()); + await expect(context.loadAssetsFromLocal()).to.be.eventually.rejectedWith( + Error, + 'must be relative to the config directory' + ); + }); + + it('should accept page html with a path relative to the config directory', async () => { + const dir = path.join(testDataDir, 'yaml', 'pages-relative-path'); + cleanThenMkdir(dir); + + const htmlContent = 'login page'; + const yaml = ` + pages: + - name: "login" + html: "page.html" + `; + const yamlFile = path.join(dir, 'config.yaml'); + fs.writeFileSync(yamlFile, yaml); + fs.writeFileSync(path.join(dir, 'page.html'), htmlContent); + + const context = new Context({ AUTH0_INPUT_FILE: yamlFile }, mockMgmtClient()); + await context.loadAssetsFromLocal(); + expect(context.assets.pages[0].html).to.equal(htmlContent); + }); + it('should dump pages', async () => { const dir = path.join(testDataDir, 'yaml', 'pagesDump'); cleanThenMkdir(dir);