Skip to content
Permalink
Browse files Browse the repository at this point in the history
Add validation to prevent docs_dir from being an absolute path
* Adds a new validation function to helpers to prevent generation when mkdocs.yml is not present or is invalid
* Handles vulnerability where docs_dir can be put in as an absolute path which copies and exposes the files from that absolute path in the file system

Signed-off-by: Jussi Hallila <jussi@hallila.com>
  • Loading branch information
Xantier committed May 27, 2021
1 parent f2cb8c8 commit 8cefadc
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/gold-kings-jog.md
@@ -0,0 +1,5 @@
---
'@backstage/techdocs-common': patch
---

Adding validation to mkdocs.yml parsing to prevent directory tree traversing
@@ -0,0 +1,3 @@
site_name: Test site name
site_description: Test site description
docs_dir: /etc
28 changes: 28 additions & 0 deletions packages/techdocs-common/src/stages/generate/helpers.test.ts
Expand Up @@ -27,6 +27,7 @@ import {
isValidRepoUrlForMkdocs,
patchMkdocsYmlPreBuild,
storeEtagMetadata,
validateMkdocsYaml,
} from './helpers';

const mockEntity = {
Expand All @@ -43,6 +44,9 @@ const mkdocsYml = fs.readFileSync(
const mkdocsYmlWithRepoUrl = fs.readFileSync(
resolvePath(__filename, '../__fixtures__/mkdocs_with_repo_url.yml'),
);
const mkdocsYmlWithInvalidDocDir = fs.readFileSync(
resolvePath(__filename, '../__fixtures__/mkdocs_invalid_doc_dir.yml'),
);
const mockLogger = getVoidLogger();
const rootDir = os.platform() === 'win32' ? 'C:\\rootDir' : '/rootDir';

Expand Down Expand Up @@ -300,4 +304,28 @@ describe('helpers', () => {
expect(json.etag).toBe('etag123abc');
});
});

describe('validateMkdocsYaml', () => {
beforeEach(() => {
mockFs({
'/mkdocs.yml': mkdocsYml,
'/mkdocs_with_repo_url.yml': mkdocsYmlWithRepoUrl,
'/mkdocs_invalid_doc_dir.yml': mkdocsYmlWithInvalidDocDir,
});
});

afterEach(() => {
mockFs.restore();
});

it('should return true on when no docs_dir present', async () => {
await expect(validateMkdocsYaml('/mkdocs.yml')).resolves.toBeUndefined();
});

it('should return false on absolute doc_dir path', async () => {
await expect(
validateMkdocsYaml('/mkdocs_invalid_doc_dir.yml'),
).rejects.toThrow();
});
});
});
25 changes: 25 additions & 0 deletions packages/techdocs-common/src/stages/generate/helpers.ts
Expand Up @@ -16,6 +16,7 @@

import { Entity } from '@backstage/catalog-model';
import { spawn } from 'child_process';
import { isAbsolute, normalize } from 'path';
import fs from 'fs-extra';
import yaml from 'js-yaml';
import { PassThrough, Writable } from 'stream';
Expand Down Expand Up @@ -138,6 +139,30 @@ export const getRepoUrlFromLocationAnnotation = (
return undefined;
};

/**
* Validating mkdocs config file for incorrect/insecure values
* Throws on invalid configs
*
* @param {string} mkdocsYmlPath Absolute path to mkdocs.yml or equivalent of a docs site
*/
export const validateMkdocsYaml = async (mkdocsYmlPath: string) => {
let mkdocsYmlFileString;
try {
mkdocsYmlFileString = await fs.readFile(mkdocsYmlPath, 'utf8');
} catch (error) {
throw new Error(
`Could not read MkDocs YAML config file ${mkdocsYmlPath} before for validation: ${error.message}`,
);
}

const mkdocsYml: any = yaml.load(mkdocsYmlFileString);
if (mkdocsYml.docs_dir && isAbsolute(normalize(mkdocsYml.docs_dir))) {
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.",
);
}
};

/**
* Update the mkdocs.yml file before TechDocs generator uses it to generate docs site.
*
Expand Down
6 changes: 5 additions & 1 deletion packages/techdocs-common/src/stages/generate/techdocs.ts
Expand Up @@ -24,6 +24,7 @@ import {
patchMkdocsYmlPreBuild,
runCommand,
storeEtagMetadata,
validateMkdocsYaml,
} from './helpers';
import { GeneratorBase, GeneratorRunOptions } from './types';

Expand Down Expand Up @@ -79,14 +80,17 @@ export class TechdocsGenerator implements GeneratorBase {
// TODO: In future mkdocs.yml can be mkdocs.yaml. So, use a config variable here to find out
// the correct file name.
// Do some updates to mkdocs.yml before generating docs e.g. adding repo_url
const mkdocsYmlPath = path.join(inputDir, 'mkdocs.yml');
if (parsedLocationAnnotation) {
await patchMkdocsYmlPreBuild(
path.join(inputDir, 'mkdocs.yml'),
mkdocsYmlPath,
this.logger,
parsedLocationAnnotation,
);
}

await validateMkdocsYaml(mkdocsYmlPath);

// Directories to bind on container
const mountDirs = {
[inputDir]: '/input',
Expand Down

0 comments on commit 8cefadc

Please sign in to comment.