Skip to content

Commit c17c0fc

Browse files
committed
Add check against directory traversal for docs_dir config value
Signed-off-by: Jussi Hallila <jussi@hallila.com>
1 parent 1d2d530 commit c17c0fc

File tree

5 files changed

+34
-8
lines changed

5 files changed

+34
-8
lines changed

.changeset/cold-rockets-mix.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@backstage/techdocs-common': patch
3+
---
4+
5+
Adding additional checks on tech docs to prevent folder traversal via mkdocs.yml docs_dir value.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
site_name: Test site name
2+
site_description: Test site description
3+
docs_dir: ../../etc/

packages/techdocs-common/src/stages/generate/helpers.test.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,19 +344,28 @@ describe('helpers', () => {
344344
mockFs.restore();
345345
});
346346

347+
const inputDir = resolvePath(__filename, '../__fixtures__/');
347348
it('should return true on when no docs_dir present', async () => {
348-
await expect(validateMkdocsYaml('/mkdocs.yml')).resolves.toBeUndefined();
349+
await expect(
350+
validateMkdocsYaml(inputDir, '/mkdocs.yml'),
351+
).resolves.toBeUndefined();
349352
});
350353

351354
it('should return false on absolute doc_dir path', async () => {
352355
await expect(
353-
validateMkdocsYaml('/mkdocs_invalid_doc_dir.yml'),
356+
validateMkdocsYaml(inputDir, '/mkdocs_invalid_doc_dir.yml'),
357+
).rejects.toThrow();
358+
});
359+
360+
it('should return false on doc_dir path that traverses directory structure backwards', async () => {
361+
await expect(
362+
validateMkdocsYaml(inputDir, '/mkdocs_invalid_doc_dir2.yml'),
354363
).rejects.toThrow();
355364
});
356365

357366
it('should validate files with custom yaml tags', async () => {
358367
await expect(
359-
validateMkdocsYaml('/mkdocs_with_extensions.yml'),
368+
validateMkdocsYaml(inputDir, '/mkdocs_with_extensions.yml'),
360369
).resolves.toBeUndefined();
361370
});
362371
});

packages/techdocs-common/src/stages/generate/helpers.ts

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ import { Entity } from '@backstage/catalog-model';
1818
import { spawn } from 'child_process';
1919
import fs from 'fs-extra';
2020
import yaml, { DEFAULT_SCHEMA, Type } from 'js-yaml';
21-
import { isAbsolute, normalize } from 'path';
2221
import { PassThrough, Writable } from 'stream';
2322
import { Logger } from 'winston';
2423
import { ParsedLocationAnnotation } from '../../helpers';
2524
import { RemoteProtocol } from '../prepare/types';
2625
import { SupportedGeneratorKey } from './types';
26+
import { resolve as resolvePath } from 'path';
2727

2828
// TODO: Implement proper support for more generators.
2929
export function getGeneratorKey(entity: Entity): SupportedGeneratorKey {
@@ -158,9 +158,13 @@ const MKDOCS_SCHEMA = DEFAULT_SCHEMA.extend([
158158
* Validating mkdocs config file for incorrect/insecure values
159159
* Throws on invalid configs
160160
*
161+
* @param {string} inputDir base dir to be used as a docs_dir path validity check
161162
* @param {string} mkdocsYmlPath Absolute path to mkdocs.yml or equivalent of a docs site
162163
*/
163-
export const validateMkdocsYaml = async (mkdocsYmlPath: string) => {
164+
export const validateMkdocsYaml = async (
165+
inputDir: string,
166+
mkdocsYmlPath: string,
167+
) => {
164168
let mkdocsYmlFileString;
165169
try {
166170
mkdocsYmlFileString = await fs.readFile(mkdocsYmlPath, 'utf8');
@@ -173,9 +177,14 @@ export const validateMkdocsYaml = async (mkdocsYmlPath: string) => {
173177
const mkdocsYml: any = yaml.load(mkdocsYmlFileString, {
174178
schema: MKDOCS_SCHEMA,
175179
});
176-
if (mkdocsYml.docs_dir && isAbsolute(normalize(mkdocsYml.docs_dir))) {
180+
181+
if (
182+
mkdocsYml.docs_dir &&
183+
!resolvePath(inputDir, mkdocsYml.docs_dir).startsWith(inputDir)
184+
) {
177185
throw new Error(
178-
"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.",
186+
`docs_dir configuration value in mkdocs can't be an absolute directory or start with ../ for security reasons.
187+
Use relative paths instead which are resolved relative to your mkdocs.yml file location.`,
179188
);
180189
}
181190
};

packages/techdocs-common/src/stages/generate/techdocs.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ export class TechdocsGenerator implements GeneratorBase {
8989
);
9090
}
9191

92-
await validateMkdocsYaml(mkdocsYmlPath);
92+
await validateMkdocsYaml(inputDir, mkdocsYmlPath);
9393

9494
// Directories to bind on container
9595
const mountDirs = {

0 commit comments

Comments
 (0)