diff --git a/.changeset/cold-rockets-mix.md b/.changeset/cold-rockets-mix.md new file mode 100644 index 0000000000..f38f5f61e9 --- /dev/null +++ b/.changeset/cold-rockets-mix.md @@ -0,0 +1,5 @@ +--- +'@backstage/techdocs-common': patch +--- + +Adding additional checks on tech docs to prevent folder traversal via mkdocs.yml docs_dir value. diff --git a/packages/techdocs-common/src/stages/generate/__fixtures__/mkdocs_invalid_doc_dir2.yml b/packages/techdocs-common/src/stages/generate/__fixtures__/mkdocs_invalid_doc_dir2.yml new file mode 100644 index 0000000000..514939e18c --- /dev/null +++ b/packages/techdocs-common/src/stages/generate/__fixtures__/mkdocs_invalid_doc_dir2.yml @@ -0,0 +1,3 @@ +site_name: Test site name +site_description: Test site description +docs_dir: ../../etc/ diff --git a/packages/techdocs-common/src/stages/generate/helpers.test.ts b/packages/techdocs-common/src/stages/generate/helpers.test.ts index 1ecb6252e5..9db76c8485 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.test.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.test.ts @@ -344,19 +344,28 @@ describe('helpers', () => { mockFs.restore(); }); + const inputDir = resolvePath(__filename, '../__fixtures__/'); it('should return true on when no docs_dir present', async () => { - await expect(validateMkdocsYaml('/mkdocs.yml')).resolves.toBeUndefined(); + await expect( + validateMkdocsYaml(inputDir, '/mkdocs.yml'), + ).resolves.toBeUndefined(); }); it('should return false on absolute doc_dir path', async () => { await expect( - validateMkdocsYaml('/mkdocs_invalid_doc_dir.yml'), + validateMkdocsYaml(inputDir, '/mkdocs_invalid_doc_dir.yml'), + ).rejects.toThrow(); + }); + + it('should return false on doc_dir path that traverses directory structure backwards', async () => { + await expect( + validateMkdocsYaml(inputDir, '/mkdocs_invalid_doc_dir2.yml'), ).rejects.toThrow(); }); it('should validate files with custom yaml tags', async () => { await expect( - validateMkdocsYaml('/mkdocs_with_extensions.yml'), + validateMkdocsYaml(inputDir, '/mkdocs_with_extensions.yml'), ).resolves.toBeUndefined(); }); }); diff --git a/packages/techdocs-common/src/stages/generate/helpers.ts b/packages/techdocs-common/src/stages/generate/helpers.ts index 3a65591c46..93915480dd 100644 --- a/packages/techdocs-common/src/stages/generate/helpers.ts +++ b/packages/techdocs-common/src/stages/generate/helpers.ts @@ -18,12 +18,12 @@ import { Entity } from '@backstage/catalog-model'; import { spawn } from 'child_process'; import fs from 'fs-extra'; import yaml, { DEFAULT_SCHEMA, Type } from 'js-yaml'; -import { isAbsolute, normalize } from 'path'; import { PassThrough, Writable } from 'stream'; import { Logger } from 'winston'; import { ParsedLocationAnnotation } from '../../helpers'; import { RemoteProtocol } from '../prepare/types'; import { SupportedGeneratorKey } from './types'; +import { resolve as resolvePath } from 'path'; // TODO: Implement proper support for more generators. export function getGeneratorKey(entity: Entity): SupportedGeneratorKey { @@ -158,9 +158,13 @@ const MKDOCS_SCHEMA = DEFAULT_SCHEMA.extend([ * Validating mkdocs config file for incorrect/insecure values * Throws on invalid configs * + * @param {string} inputDir base dir to be used as a docs_dir path validity check * @param {string} mkdocsYmlPath Absolute path to mkdocs.yml or equivalent of a docs site */ -export const validateMkdocsYaml = async (mkdocsYmlPath: string) => { +export const validateMkdocsYaml = async ( + inputDir: string, + mkdocsYmlPath: string, +) => { let mkdocsYmlFileString; try { mkdocsYmlFileString = await fs.readFile(mkdocsYmlPath, 'utf8'); @@ -173,9 +177,14 @@ export const validateMkdocsYaml = async (mkdocsYmlPath: string) => { const mkdocsYml: any = yaml.load(mkdocsYmlFileString, { schema: MKDOCS_SCHEMA, }); - if (mkdocsYml.docs_dir && isAbsolute(normalize(mkdocsYml.docs_dir))) { + + if ( + mkdocsYml.docs_dir && + !resolvePath(inputDir, mkdocsYml.docs_dir).startsWith(inputDir) + ) { throw new Error( - "docs_dir configuration value in mkdocs can't be an absolute directory path for security reasons. Use relative paths instead which are resolved relative to your mkdocs.yml file location.", + `docs_dir configuration value in mkdocs can't be an absolute directory or start with ../ for security reasons. + Use relative paths instead which are resolved relative to your mkdocs.yml file location.`, ); } }; diff --git a/packages/techdocs-common/src/stages/generate/techdocs.ts b/packages/techdocs-common/src/stages/generate/techdocs.ts index 1f2a9ae3ff..be9f494dc7 100644 --- a/packages/techdocs-common/src/stages/generate/techdocs.ts +++ b/packages/techdocs-common/src/stages/generate/techdocs.ts @@ -89,7 +89,7 @@ export class TechdocsGenerator implements GeneratorBase { ); } - await validateMkdocsYaml(mkdocsYmlPath); + await validateMkdocsYaml(inputDir, mkdocsYmlPath); // Directories to bind on container const mountDirs = {